Refactor dirent parsing utilies.

- Added `fsutil.ForEachDirent()` (used by directfs).
- Added `fsutil.DirentNames()`, which uses `ForEachDirent()`. Used by sysfs
  and in the future will be used by nvproxy/tpuproxy.

Separately, fixed some bugs in sysfs:
- We were leaking FDs in `hostDirEntries()`.
- We were leaking FD in `hostFile.Generate()` on error path.
- Got rid of `hostFileBufSize` users. They expected contents to be of 4096
  bytes only. Instead made the functions agnostic of file size.

PiperOrigin-RevId: 580688250

pkg/fsutil/fsutil.go

@@ -15,3 +15,34 @@
 // Package fsutil contains filesystem utilities that can be shared between the
 // sentry and other sandbox components.
 package fsutil
+
+import "golang.org/x/sys/unix"
+
+// DirentHandler is a function that handles a dirent.
+type DirentHandler func(ino uint64, off int64, ftype uint8, name string, reclen uint16)
+
+// ForEachDirent retrieves all dirents from dirfd using getdents64(2) and
+// invokes handleDirent on them.
+func ForEachDirent(dirfd int, handleDirent DirentHandler) error {
+	var direntsBuf [8192]byte
+	for {
+		n, err := unix.Getdents(dirfd, direntsBuf[:])
+		if err != nil {
+			return err
+		}
+		if n <= 0 {
+			return nil
+		}
+		ParseDirents(direntsBuf[:n], handleDirent)
+	}
+}
+
+// DirentNames retrieves all dirents from dirfd using getdents64(2) and returns
+// all the recorded dirent names.
+func DirentNames(dirfd int) ([]string, error) {
+	var names []string
+	err := ForEachDirent(dirfd, func(_ uint64, _ int64, _ uint8, name string, _ uint16) {
+		names = append(names, name)
+	})
+	return names, err
+}

pkg/fsutil/fsutil_unsafe.go

@@ -90,7 +90,7 @@ func RenameAt(oldDirFD int, oldName string, newDirFD int, newName string) error
 
 // ParseDirents parses dirents from buf. buf must have been populated by
 // getdents64(2) syscall. It calls the handleDirent callback for each dirent.
-func ParseDirents(buf []byte, handleDirent func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool) {
+func ParseDirents(buf []byte, handleDirent DirentHandler) {
 	for len(buf) > 0 {
 		// Interpret the buf populated by unix.Getdents as unix.Dirent.
 		dirent := *(*unix.Dirent)(unsafe.Pointer(&buf[0]))
@@ -118,8 +118,6 @@ func ParseDirents(buf []byte, handleDirent func(ino uint64, off int64, ftype uin
 		}
 
 		// Deliver results to caller.
-		if !handleDirent(dirent.Ino, dirent.Off, dirent.Type, name, dirent.Reclen) {
-			return
-		}
+		handleDirent(dirent.Ino, dirent.Off, dirent.Type, name, dirent.Reclen)
 	}
 }

pkg/sentry/fsimpl/gofer/directfs_dentry.go

@@ -565,29 +565,17 @@ func (d *directfsDentry) getDirentsLocked(recordDirent func(name string, key ino
 		return err
 	}
 
-	var direntsBuf [8192]byte
-	for {
-		n, err := unix.Getdents(readFD, direntsBuf[:])
+	return fsutil.ForEachDirent(readFD, func(ino uint64, off int64, ftype uint8, name string, reclen uint16) {
+		// We also want the device ID, which annoyingly incurs an additional
+		// syscall per dirent.
+		// TODO(gvisor.dev/issue/6665): Get rid of per-dirent stat.
+		stat, err := fsutil.StatAt(d.controlFD, name)
 		if err != nil {
-			return err
+			log.Warningf("Getdent64: skipping file %q with failed stat, err: %v", path.Join(genericDebugPathname(&d.dentry), name), err)
+			return
 		}
-		if n <= 0 {
-			return nil
-		}
-
-		fsutil.ParseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool {
-			// We also want the device ID, which annoyingly incurs an additional
-			// syscall per dirent.
-			// TODO(gvisor.dev/issue/6665): Get rid of per-dirent stat.
-			stat, err := fsutil.StatAt(d.controlFD, name)
-			if err != nil {
-				log.Warningf("Getdent64: skipping file %q with failed stat, err: %v", path.Join(genericDebugPathname(&d.dentry), name), err)
-				return true
-			}
-			recordDirent(name, inoKeyFromStat(&stat), ftype)
-			return true
-		})
-	}
+		recordDirent(name, inoKeyFromStat(&stat), ftype)
+	})
 }
 
 // Precondition: fs.renameMu is locked.

pkg/sentry/fsimpl/sys/pci.go

@@ -27,12 +27,7 @@ import (
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 )
 
-const (
-	pciMainBusDevicePath = "/sys/devices/pci0000:00"
-	// Size of the buffer that host file content will be read into. All relevant
-	// host files are smaller than this.
-	hostFileBufSize = 0x1000
-)
+const pciMainBusDevicePath = "/sys/devices/pci0000:00"
 
 var (
 	// Matches PCI device addresses in the main domain.
@@ -150,15 +145,6 @@ func hostDirEntries(path string) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	var buf [hostFileBufSize]byte
-	n, err := unix.Getdents(fd, buf[:])
-	if err != nil {
-		return nil, err
-	}
-	var dents []string
-	fsutil.ParseDirents(buf[:n], func(_ uint64, _ int64, _ uint8, name string, _ uint16) bool {
-		dents = append(dents, name)
-		return true
-	})
-	return dents, nil
+	defer unix.Close(fd)
+	return fsutil.DirentNames(fd)
 }

pkg/sentry/fsimpl/sys/sys.go

@@ -18,6 +18,7 @@ package sys
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"strconv"
 
 	"golang.org/x/sys/unix"
@@ -331,14 +332,10 @@ func (hf *hostFile) Generate(ctx context.Context, buf *bytes.Buffer) error {
 	if err != nil {
 		return err
 	}
-	var data [hostFileBufSize]byte
-	n, err := unix.Read(fd, data[:])
-	if err != nil {
-		return err
-	}
-	unix.Close(fd)
-	buf.Write(data[:n])
-	return nil
+	file := os.NewFile(uintptr(fd), hf.hostPath)
+	defer file.Close()
+	_, err = buf.ReadFrom(file)
+	return err
 }
 
 func (fs *filesystem) newHostFile(ctx context.Context, creds *auth.Credentials, mode linux.FileMode, hostPath string) kernfs.Inode {

runsc/fsgofer/lisafs.go

@@ -1020,7 +1020,7 @@ func (fd *openFDLisa) Getdent64(count uint32, seek0 bool, recordDirent func(lisa
 			break
 		}
 
-		fsutil.ParseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool {
+		fsutil.ParseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string, reclen uint16) {
 			dirent := lisafs.Dirent64{
 				Ino:  primitive.Uint64(ino),
 				Off:  primitive.Uint64(off),
@@ -1034,13 +1034,12 @@ func (fd *openFDLisa) Getdent64(count uint32, seek0 bool, recordDirent func(lisa
 			stat, err := fsutil.StatAt(fd.hostFD, name)
 			if err != nil {
 				log.Warningf("Getdent64: skipping file %q with failed stat, err: %v", path.Join(fd.ControlFD().FD().Node().FilePath(), name), err)
-				return true
+				return
 			}
 			dirent.DevMinor = primitive.Uint32(unix.Minor(stat.Dev))
 			dirent.DevMajor = primitive.Uint32(unix.Major(stat.Dev))
 			recordDirent(dirent)
 			bytesRead += int(reclen)
-			return true
 		})
 	}
 	return nil