diff --git a/pkg/lisafs/client.go b/pkg/lisafs/client.go index 4e4f12513..92997b930 100644 --- a/pkg/lisafs/client.go +++ b/pkg/lisafs/client.go @@ -72,7 +72,7 @@ type Client struct { // the server and creates channels for fast IPC. NewClient takes ownership over // the passed socket. On success, it returns the initialized client along with // the root Inode. -func NewClient(sock *unet.Socket) (*Client, *Inode, error) { +func NewClient(sock *unet.Socket) (*Client, Inode, error) { maxChans := maxChannels() c := &Client{ sockComm: newSockComm(sock), @@ -99,7 +99,7 @@ func NewClient(sock *unet.Socket) (*Client, *Inode, error) { c.supported[Mount] = true var mountResp MountResp if err := c.SndRcvMessage(Mount, 0, NoopMarshal, mountResp.CheckedUnmarshal, nil); err != nil { - return nil, nil, err + return nil, Inode{}, err } // Initialize client. @@ -142,12 +142,12 @@ func NewClient(sock *unet.Socket) (*Client, *Inode, error) { for _, channelErr := range channelErrs { // Return the first non-nil channel creation error. if channelErr != nil { - return nil, nil, channelErr + return nil, Inode{}, channelErr } } cu.Release() - return c, &mountResp.Root, nil + return c, mountResp.Root, nil } func (c *Client) watchdog() { diff --git a/pkg/lisafs/client_file.go b/pkg/lisafs/client_file.go index 1682eb2c2..6ec2a657a 100644 --- a/pkg/lisafs/client_file.go +++ b/pkg/lisafs/client_file.go @@ -212,7 +212,7 @@ func (f *ClientFD) Write(ctx context.Context, src []byte, offset uint64) (uint64 } // MkdirAt makes the MkdirAt RPC. -func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID) (*Inode, error) { +func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID) (Inode, error) { var req MkdirAtReq req.DirFD = f.fd req.Name = SizedString(name) @@ -224,11 +224,11 @@ func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(MkdirAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil) ctx.UninterruptibleSleepFinish(false) - return &resp.ChildDir, err + return resp.ChildDir, err } // SymlinkAt makes the SymlinkAt RPC. -func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID, gid GID) (*Inode, error) { +func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID, gid GID) (Inode, error) { req := SymlinkAtReq{ DirFD: f.fd, Name: SizedString(name), @@ -241,11 +241,11 @@ func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID, ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(SymlinkAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil) ctx.UninterruptibleSleepFinish(false) - return &resp.Symlink, err + return resp.Symlink, err } // LinkAt makes the LinkAt RPC. -func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (*Inode, error) { +func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (Inode, error) { req := LinkAtReq{ DirFD: f.fd, Target: targetFD, @@ -256,11 +256,11 @@ func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (*Ino ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(LinkAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil) ctx.UninterruptibleSleepFinish(false) - return &resp.Link, err + return resp.Link, err } // MknodAt makes the MknodAt RPC. -func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID, minor, major uint32) (*Inode, error) { +func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID, minor, major uint32) (Inode, error) { var req MknodAtReq req.DirFD = f.fd req.Name = SizedString(name) @@ -274,7 +274,7 @@ func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(MknodAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil) ctx.UninterruptibleSleepFinish(false) - return &resp.Child, err + return resp.Child, err } // SetStat makes the SetStat RPC. @@ -318,7 +318,7 @@ func (f *ClientFD) WalkMultiple(ctx context.Context, names []string) (WalkStatus } // Walk makes the Walk RPC with just one path component to walk. -func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) { +func (f *ClientFD) Walk(ctx context.Context, name string) (Inode, error) { req := WalkReq{ DirFD: f.fd, Path: []string{name}, @@ -330,15 +330,15 @@ func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) { err := f.client.SndRcvMessage(Walk, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil) ctx.UninterruptibleSleepFinish(false) if err != nil { - return nil, err + return Inode{}, err } switch resp.Status { case WalkComponentDoesNotExist: - return nil, unix.ENOENT + return Inode{}, unix.ENOENT case WalkComponentSymlink: // f is not a directory which can be walked on. - return nil, unix.ENOTDIR + return Inode{}, unix.ENOTDIR } if n := len(resp.Inodes); n > 1 { @@ -346,12 +346,12 @@ func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) { f.client.CloseFDBatched(ctx, resp.Inodes[i].ControlFD) } log.Warningf("requested to walk one component, but got %d results", n) - return nil, unix.EIO + return Inode{}, unix.EIO } else if n == 0 { log.Warningf("walk has success status but no results returned") - return nil, unix.ENOENT + return Inode{}, unix.ENOENT } - return &inode[0], err + return inode[0], err } // WalkStat makes the WalkStat RPC with multiple path components to walk. diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index d99a6112c..7224ad09b 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -22,6 +22,7 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" + "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/p9" "gvisor.dev/gvisor/pkg/refsvfs2" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -35,6 +36,25 @@ func (d *dentry) isDir() bool { return d.fileType() == linux.S_IFDIR } +// Preconditions: +// - filesystem.renameMu must be locked. +// - d.dirMu must be locked. +// - d.isDir(). +// - child must be a newly-created dentry that has never had a parent. +func (d *dentry) insertCreatedChildLocked(ctx context.Context, childIno *lisafs.Inode, childName string, updateChild func(child *dentry), ds **[]*dentry) error { + child, err := d.fs.newDentryLisa(ctx, childIno) + if err != nil { + d.fs.clientLisa.CloseFDBatched(ctx, childIno.ControlFD) + return err + } + d.cacheNewChildLocked(child, childName) + appendNewChildDentry(ds, d, child) + if updateChild != nil { + updateChild(child) + } + return nil +} + // Preconditions: // * filesystem.renameMu must be locked. // * d.dirMu must be locked. diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 16b8e2e3a..a57fa994b 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -388,7 +388,7 @@ func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name s return nil, err } // Create a new dentry representing the file. - child, err = fs.newDentryLisa(ctx, childInode) + child, err = fs.newDentryLisa(ctx, &childInode) if err != nil { fs.clientLisa.CloseFDBatched(ctx, childInode.ControlFD) return nil, err @@ -482,7 +482,7 @@ func (fs *filesystem) resolveLocked(ctx context.Context, rp *vfs.ResolvingPath, // Preconditions: // * !rp.Done(). // * For the final path component in rp, !rp.ShouldFollowSymlink(). -func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error), createInSyntheticDir func(parent *dentry, name string) error, updateChild func(child *dentry)) error { +func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) error, createInSyntheticDir func(parent *dentry, name string) error) error { var ds *[]*dentry fs.renameMu.RLock() defer fs.renameMuRUnlockAndCheckCaching(ctx, &ds) @@ -569,26 +569,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir // No cached dentry exists; however, in InteropModeShared there might still be // an existing file at name. Just attempt the file creation RPC anyways. If a // file does exist, the RPC will fail with EEXIST like we would have. - lisaInode, err := createInRemoteDir(parent, name, &ds) - if err != nil { + if err := createInRemoteDir(parent, name, &ds); err != nil { return err } - // lisafs may aggresively cache newly created inodes. This has helped reduce - // Walk RPCs in practice. - if lisaInode != nil { - child, err := fs.newDentryLisa(ctx, lisaInode) - if err != nil { - fs.clientLisa.CloseFDBatched(ctx, lisaInode.ControlFD) - return err - } - parent.cacheNewChildLocked(child, name) - appendNewChildDentry(&ds, parent, child) - - // lisafs may update dentry properties upon successful creation. - if updateChild != nil { - updateChild(child) - } - } if fs.opts.interop != InteropModeShared { if child, ok := parent.children[name]; ok && child == nil { // Delete the now-stale negative dentry. @@ -833,31 +816,35 @@ func (fs *filesystem) GetParentDentryAt(ctx context.Context, rp *vfs.ResolvingPa // LinkAt implements vfs.FilesystemImpl.LinkAt. func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs.VirtualDentry) error { - err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, childName string, ds **[]*dentry) (*lisafs.Inode, error) { + err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, childName string, ds **[]*dentry) error { if rp.Mount() != vd.Mount() { - return nil, linuxerr.EXDEV + return linuxerr.EXDEV } d := vd.Dentry().Impl().(*dentry) if d.isDir() { - return nil, linuxerr.EPERM + return linuxerr.EPERM } gid := auth.KGID(atomic.LoadUint32(&d.gid)) uid := auth.KUID(atomic.LoadUint32(&d.uid)) mode := linux.FileMode(atomic.LoadUint32(&d.mode)) if err := vfs.MayLink(rp.Credentials(), mode, uid, gid); err != nil { - return nil, err + return err } if d.nlink == 0 { - return nil, linuxerr.ENOENT + return linuxerr.ENOENT } if d.nlink == math.MaxUint32 { - return nil, linuxerr.EMLINK + return linuxerr.EMLINK } if fs.opts.lisaEnabled { - return parent.controlFDLisa.LinkAt(ctx, d.controlFDLisa.ID(), childName) + linkInode, err := parent.controlFDLisa.LinkAt(ctx, d.controlFDLisa.ID(), childName) + if err != nil { + return err + } + return parent.insertCreatedChildLocked(ctx, &linkInode, childName, nil, ds) } - return nil, parent.file.link(ctx, d.file, childName) - }, nil, nil) + return parent.file.link(ctx, d.file, childName) + }, nil) if err == nil { // Success! @@ -869,7 +856,7 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. // MkdirAt implements vfs.FilesystemImpl.MkdirAt. func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MkdirOptions) error { creds := rp.Credentials() - return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) { + return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { // If the parent is a setgid directory, use the parent's GID // rather than the caller's and enable setgid. kgid := creds.EffectiveKGID @@ -878,12 +865,15 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v kgid = auth.KGID(atomic.LoadUint32(&parent.gid)) mode |= linux.S_ISGID } - var ( - childDirInode *lisafs.Inode - err error - ) + var err error if fs.opts.lisaEnabled { + var childDirInode lisafs.Inode childDirInode, err = parent.controlFDLisa.MkdirAt(ctx, name, mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(kgid)) + if err == nil { + if err = parent.insertCreatedChildLocked(ctx, &childDirInode, name, nil, ds); err != nil { + return err + } + } } else { _, err = parent.file.mkdir(ctx, name, p9.FileMode(mode), (p9.UID)(creds.EffectiveKUID), p9.GID(kgid)) } @@ -891,11 +881,11 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v if fs.opts.interop != InteropModeShared { parent.incLinks() } - return childDirInode, nil + return nil } if !opts.ForSyntheticMountpoint || linuxerr.Equals(linuxerr.EEXIST, err) { - return nil, err + return err } ctx.Infof("Failed to create remote directory %q: %v; falling back to synthetic directory", name, err) parent.createSyntheticChildLocked(&createSyntheticOpts{ @@ -908,7 +898,7 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v if fs.opts.interop != InteropModeShared { parent.incLinks() } - return nil, nil + return nil }, func(parent *dentry, name string) error { if !opts.ForSyntheticMountpoint { // Can't create non-synthetic files in synthetic directories. @@ -922,26 +912,27 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v }) parent.incLinks() return nil - }, nil) + }) } // MknodAt implements vfs.FilesystemImpl.MknodAt. func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MknodOptions) error { - return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) { + return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { creds := rp.Credentials() - var ( - childInode *lisafs.Inode - err error - ) + var err error if fs.opts.lisaEnabled { + var childInode lisafs.Inode childInode, err = parent.controlFDLisa.MknodAt(ctx, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID), opts.DevMinor, opts.DevMajor) + if err == nil { + return parent.insertCreatedChildLocked(ctx, &childInode, name, nil, ds) + } } else { _, err = parent.file.mknod(ctx, name, (p9.FileMode)(opts.Mode), opts.DevMajor, opts.DevMinor, (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID)) } if err == nil { - return childInode, nil + return nil } else if !linuxerr.Equals(linuxerr.EPERM, err) { - return nil, err + return err } // EPERM means that gofer does not allow creating a socket or pipe. Fallback @@ -952,10 +943,10 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v switch { case err == nil: // Step succeeded, another file exists. - return nil, linuxerr.EEXIST + return linuxerr.EEXIST case !linuxerr.Equals(linuxerr.ENOENT, err): // Unexpected error. - return nil, err + return err } switch opts.Mode.FileType() { @@ -968,7 +959,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v endpoint: opts.Endpoint, }) *ds = appendDentry(*ds, parent) - return nil, nil + return nil case linux.S_IFIFO: parent.createSyntheticChildLocked(&createSyntheticOpts{ name: name, @@ -978,11 +969,11 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize), }) *ds = appendDentry(*ds, parent) - return nil, nil + return nil } // Retain error from gofer if synthetic file cannot be created internally. - return nil, linuxerr.EPERM - }, nil, nil) + return linuxerr.EPERM + }, nil) } // OpenAt implements vfs.FilesystemImpl.OpenAt. @@ -1740,21 +1731,25 @@ func (fs *filesystem) StatFSAt(ctx context.Context, rp *vfs.ResolvingPath) (linu // SymlinkAt implements vfs.FilesystemImpl.SymlinkAt. func (fs *filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, target string) error { - return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) { + return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { creds := rp.Credentials() if fs.opts.lisaEnabled { - return parent.controlFDLisa.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) + symlinkInode, err := parent.controlFDLisa.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) + if err != nil { + return err + } + return parent.insertCreatedChildLocked(ctx, &symlinkInode, name, func(child *dentry) { + if fs.opts.interop != InteropModeShared { + // lisafs caches the symlink target on creation. In practice, this + // helps avoid a lot of ReadLink RPCs. + child.haveTarget = true + child.target = target + } + }, ds) } _, err := parent.file.symlink(ctx, target, name, (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID)) - return nil, err - }, nil, func(child *dentry) { - if fs.opts.interop != InteropModeShared { - // lisafs caches the symlink target on creation. In practice, this - // helps avoid a lot of ReadLink RPCs. - child.haveTarget = true - child.target = target - } - }) + return err + }, nil) } // UnlinkAt implements vfs.FilesystemImpl.UnlinkAt. diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 3fd65ee7c..310bcd5d6 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -502,12 +502,12 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt func (fs *filesystem) initClientAndRoot(ctx context.Context) error { var err error if fs.opts.lisaEnabled { - var rootInode *lisafs.Inode + var rootInode lisafs.Inode rootInode, err = fs.initClientLisa(ctx) if err != nil { return err } - fs.root, err = fs.newDentryLisa(ctx, rootInode) + fs.root, err = fs.newDentryLisa(ctx, &rootInode) if err != nil { fs.clientLisa.CloseFDBatched(ctx, rootInode.ControlFD) } @@ -524,18 +524,18 @@ func (fs *filesystem) initClientAndRoot(ctx context.Context) error { return err } -func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error) { +func (fs *filesystem) initClientLisa(ctx context.Context) (lisafs.Inode, error) { sock, err := unet.NewSocket(fs.opts.fd) if err != nil { - return nil, err + return lisafs.Inode{}, err } - var rootInode *lisafs.Inode + var rootInode lisafs.Inode ctx.UninterruptibleSleepStart(false) fs.clientLisa, rootInode, err = lisafs.NewClient(sock) ctx.UninterruptibleSleepFinish(false) if err != nil { - return nil, err + return lisafs.Inode{}, err } if fs.opts.aname == "/" { return rootInode, nil @@ -546,7 +546,7 @@ func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error) status, inodes, err := rootFD.WalkMultiple(ctx, strings.Split(fs.opts.aname, "/")) rootFD.CloseBatched(ctx) if err != nil { - return nil, err + return lisafs.Inode{}, err } // Close all intermediate FDs to the attach point. @@ -558,12 +558,12 @@ func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error) switch status { case lisafs.WalkSuccess: - return &inodes[numInodes-1], nil + return inodes[numInodes-1], nil default: last := fs.clientLisa.NewFD(inodes[numInodes-1].ControlFD) last.CloseBatched(ctx) log.Warningf("initClientLisa failed because walk to attach point %q failed: lisafs.WalkStatus = %v", fs.opts.aname, status) - return nil, unix.ENOENT + return lisafs.Inode{}, unix.ENOENT } } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 82878c056..01f053999 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -195,7 +195,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest if err != nil { return err } - if err := fs.root.restoreFileLisa(ctx, rootInode, &opts); err != nil { + if err := fs.root.restoreFileLisa(ctx, &rootInode, &opts); err != nil { return err } } else { @@ -381,7 +381,7 @@ func (d *dentry) restoreRecursive(ctx context.Context, opts *vfs.CompleteRestore if err != nil { return err } - if err := d.restoreFileLisa(ctx, inode, opts); err != nil { + if err := d.restoreFileLisa(ctx, &inode, opts); err != nil { return err } } else {