Skip to content

Commit b6342ac

Browse files
wicked-wixclaude
andauthored
Fix drive handle leak, silent ParseInt errors, dead code, and duplication (#11)
- Extract unlock() and Execute() to internal/pba to eliminate verbatim duplication across both pbainit commands - Fix defer d.Close() inside device loop (defers are function-scoped; all handles leaked until main() returned) - Fix strconv.ParseInt bitsize int8→int32 and add error handling; int8 is too small for Linux device major numbers and a malformed sysfs entry could panic on parts[1] index or silently write a wrong Mknod - Remove always-false reboot_bug variable and its dead branch (all assignments were commented out); rename pci_block→pciBlock - Replace deprecated ioutil.ReadDir/ReadFile with os.ReadDir/ReadFile Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cc243c8 commit b6342ac

3 files changed

Lines changed: 118 additions & 166 deletions

File tree

cmd/pbainit-interactive/main.go

Lines changed: 21 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
package main
22

33
import (
4-
"crypto/sha1"
54
"encoding/base64"
65
"fmt"
7-
"io/ioutil"
86
"log"
97
"os"
10-
"os/exec"
118
"path/filepath"
129
"strconv"
1310
"strings"
@@ -16,13 +13,13 @@ import (
1613

1714
tcg "github.com/open-source-firmware/go-tcg-storage/pkg/core"
1815
"github.com/open-source-firmware/go-tcg-storage/pkg/drive"
19-
"github.com/open-source-firmware/go-tcg-storage/pkg/locking"
2016
"github.com/u-root/u-root/pkg/libinit"
2117
"github.com/u-root/u-root/pkg/mount"
2218
"github.com/u-root/u-root/pkg/ulog"
23-
"golang.org/x/crypto/pbkdf2"
2419
"golang.org/x/sys/unix"
2520
"golang.org/x/term"
21+
22+
"github.com/elastx/elx-pba/internal/pba"
2623
)
2724

2825
var (
@@ -60,11 +57,11 @@ func main() {
6057
defer func() {
6158
log.Printf("Starting emergency shell...")
6259
for {
63-
Execute("/bbin/elvish")
60+
pba.Execute("/bbin/elvish")
6461
}
6562
}()
6663

67-
sysblk, err := ioutil.ReadDir("/sys/class/block/")
64+
sysblk, err := os.ReadDir("/sys/class/block/")
6865
if err != nil {
6966
log.Printf("Failed to enumerate block devices: %v", err)
7067
return
@@ -79,14 +76,22 @@ func main() {
7976
}
8077
devpath := filepath.Join("/dev", devname)
8178
if _, err := os.Stat(devpath); os.IsNotExist(err) {
82-
majmin, err := ioutil.ReadFile(filepath.Join("/sys/class/block", devname, "dev"))
79+
majmin, err := os.ReadFile(filepath.Join("/sys/class/block", devname, "dev"))
8380
if err != nil {
8481
log.Printf("Failed to read major:minor for %s: %v", devname, err)
8582
continue
8683
}
8784
parts := strings.Split(strings.TrimSpace(string(majmin)), ":")
88-
major, _ := strconv.ParseInt(parts[0], 10, 8)
89-
minor, _ := strconv.ParseInt(parts[1], 10, 8)
85+
if len(parts) != 2 {
86+
log.Printf("Unexpected major:minor format for %s: %q", devname, string(majmin))
87+
continue
88+
}
89+
major, errMaj := strconv.ParseInt(parts[0], 10, 32)
90+
minor, errMin := strconv.ParseInt(parts[1], 10, 32)
91+
if errMaj != nil || errMin != nil {
92+
log.Printf("Failed to parse major:minor for %s: %v %v", devname, errMaj, errMin)
93+
continue
94+
}
9095
if err := unix.Mknod(filepath.Join("/dev", devname), unix.S_IFBLK|0600, int(major<<16|minor)); err != nil {
9196
log.Printf("Mknod(%s) failed: %v", devname, err)
9297
continue
@@ -98,7 +103,6 @@ func main() {
98103
log.Printf("drive.Open(%s): %v", devpath, err)
99104
continue
100105
}
101-
defer d.Close()
102106
identity, err := d.Identify()
103107
if err != nil {
104108
log.Printf("drive.Identify(%s): %v", devpath, err)
@@ -109,6 +113,7 @@ func main() {
109113
}
110114
d0, err := tcg.Discovery0(d)
111115
if err != nil {
116+
d.Close()
112117
if err != tcg.ErrNotSupported {
113118
log.Printf("tcg.Discovery0(%s): %v", devpath, err)
114119
}
@@ -121,17 +126,14 @@ func main() {
121126
}
122127
unlocked := false
123128
for !unlocked {
124-
// reuse-existing password for multiple drives
125129
if password == "" {
126130
password = getDrivePassword()
127131
if password == "" {
128-
// skip on empty password
129132
break
130133
}
131134
}
132-
if err := unlock(d, password, dsn); err != nil {
135+
if err := pba.Unlock(d, password, dsn); err != nil {
133136
log.Printf("Failed to unlock %s: %v", identity, err)
134-
// clear password to be queried again
135137
password = ""
136138
} else {
137139
unlocked = true
@@ -144,6 +146,7 @@ func main() {
144146
} else {
145147
log.Printf("Considered drive %s, but drive is not locked", identity)
146148
}
149+
d.Close()
147150
}
148151

149152
if startEmergencyShell {
@@ -156,13 +159,12 @@ func main() {
156159
return
157160
}
158161

159-
// reboot for now as 'boot' would mount filesystems and therefore mess up hibernation :-(
160-
// note that ext3 or ext4 will replay its journal even when mounted read-only if the filesystem is dirty
161-
Execute("/bbin/shutdown", "reboot")
162+
// reboot rather than kexec: 'boot' mounts filesystems which breaks hibernation,
163+
// and ext3/ext4 replays its journal even on read-only mounts when the fs is dirty
164+
pba.Execute("/bbin/shutdown", "reboot")
162165
}
163166

164167
func getDrivePassword() string {
165-
// avoid kernel log messages messing up prompt
166168
if err := ulog.KernelLog.SetConsoleLogLevel(ulog.KLogWarning); err != nil {
167169
log.Printf("Could not set log level KLogWarning: %v", err)
168170
}
@@ -178,41 +180,7 @@ func getDrivePassword() string {
178180
return string(bytePassword)
179181
}
180182

181-
func unlock(d tcg.DriveIntf, pass string, driveserial []byte) error {
182-
// Same format as used by sedutil for compatibility
183-
salt := fmt.Sprintf("%-20s", string(driveserial))
184-
pin := pbkdf2.Key([]byte(pass), []byte(salt[:20]), 75000, 32, sha1.New)
185-
186-
cs, lmeta, err := locking.Initialize(d)
187-
if err != nil {
188-
return fmt.Errorf("locking.Initialize: %v", err)
189-
}
190-
defer cs.Close()
191-
l, err := locking.NewSession(cs, lmeta, locking.DefaultAuthority(pin))
192-
if err != nil {
193-
return fmt.Errorf("locking.NewSession: %v", err)
194-
}
195-
defer l.Close()
196-
197-
for i, r := range l.Ranges {
198-
if err := r.UnlockRead(); err != nil {
199-
log.Printf("Read unlock range %d failed: %v", i, err)
200-
}
201-
if err := r.UnlockWrite(); err != nil {
202-
log.Printf("Write unlock range %d failed: %v", i, err)
203-
}
204-
}
205-
206-
if l.MBREnabled && !l.MBRDone {
207-
if err := l.SetMBRDone(true); err != nil {
208-
return fmt.Errorf("SetMBRDone: %v", err)
209-
}
210-
}
211-
return nil
212-
}
213-
214183
func waitForEnter(prompt string, seconds int) bool {
215-
216184
f, err := os.OpenFile("/dev/console", os.O_RDWR, 0)
217185
if err != nil {
218186
log.Printf("ERROR: Open /dev/console failed: %v", err)
@@ -247,28 +215,6 @@ func waitForEnter(prompt string, seconds int) bool {
247215
}
248216
}
249217

250-
// nobody pressed enter (need \r to reset start of line)
251218
fmt.Println("\r")
252219
return false
253220
}
254-
255-
func Execute(name string, args ...string) {
256-
environ := append(os.Environ(), "USER=root")
257-
environ = append(environ, "HOME=/root")
258-
environ = append(environ, "TZ=UTC")
259-
260-
cmd := exec.Command(name, args...)
261-
cmd.Dir = "/"
262-
cmd.Env = environ
263-
cmd.Stdin = os.Stdin
264-
cmd.Stderr = os.Stderr
265-
cmd.Stdout = os.Stdout
266-
if cmd.SysProcAttr == nil {
267-
cmd.SysProcAttr = &syscall.SysProcAttr{}
268-
}
269-
cmd.SysProcAttr.Setctty = true
270-
cmd.SysProcAttr.Setsid = true
271-
if err := cmd.Run(); err != nil {
272-
log.Printf("Failed to execute: %v", err)
273-
}
274-
}

0 commit comments

Comments
 (0)