Skip to content

Commit

Permalink
Refactor dirent parsing utilies.
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
ayushr2 authored and gvisor-bot committed Nov 8, 2023
1 parent 40ee36a commit 612e63a
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 53 deletions.
31 changes: 31 additions & 0 deletions pkg/fsutil/fsutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 2 additions & 4 deletions pkg/fsutil/fsutil_unsafe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand Down Expand Up @@ -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)
}
}
30 changes: 9 additions & 21 deletions pkg/sentry/fsimpl/gofer/directfs_dentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
if n <= 0 {
return nil
log.Warningf("Getdent64: skipping file %q with failed stat, err: %v", path.Join(genericDebugPathname(&d.dentry), name), err)
return
}

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.
Expand Down
20 changes: 3 additions & 17 deletions pkg/sentry/fsimpl/sys/pci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
13 changes: 5 additions & 8 deletions pkg/sentry/fsimpl/sys/sys.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package sys
import (
"bytes"
"fmt"
"os"
"strconv"

"golang.org/x/sys/unix"
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions runsc/fsgofer/lisafs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
Expand Down

0 comments on commit 612e63a

Please sign in to comment.