diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 8cc74dd74..851bc0f43 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -248,7 +248,7 @@ func (d *dentry) getRemoteChild(ctx context.Context, name string) (*dentry, erro // Postcondition: The returned dentry is already cached appropriately. // // +checklocksread:d.opMu -func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { +func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp resolvingPath, ds **[]*dentry) (*dentry, error) { switch dt := d.impl.(type) { case *lisafsDentry: return dt.getRemoteChildAndWalkPathLocked(ctx, rp, ds) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index fc9dec077..04c2c779d 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -177,7 +177,7 @@ func (fs *filesystem) renameMuUnlockAndCheckCaching(ctx context.Context, ds **[] // part of rp must have been revalidated. // // +checklocksread:d.opMu -func (fs *filesystem) stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, mayFollowSymlinks bool, ds **[]*dentry) (*dentry, bool, error) { +func (fs *filesystem) stepLocked(ctx context.Context, rp resolvingPath, d *dentry, mayFollowSymlinks bool, ds **[]*dentry) (*dentry, bool, error) { if !d.isDir() { return nil, false, linuxerr.ENOTDIR } @@ -293,7 +293,7 @@ func (fs *filesystem) getRemoteChildLocked(ctx context.Context, parent *dentry, // may prefetch the entire path represented by rp. // // +checklocksread:parent.opMu -func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { +func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp resolvingPath, ds **[]*dentry) (*dentry, error) { if child, err := parent.getCachedChildLocked(rp.Component()); child != nil || err != nil { return child, err } @@ -344,11 +344,12 @@ func (d *dentry) getCachedChildLocked(name string) (*dentry, error) { // - !rp.Done(). // - If !d.cachedMetadataAuthoritative(), then d's cached metadata must be up // to date. -func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, ds **[]*dentry) (*dentry, error) { - if err := fs.revalidateParentDir(ctx, rp, d, ds); err != nil { +func (fs *filesystem) walkParentDirLocked(ctx context.Context, vfsRP *vfs.ResolvingPath, d *dentry, ds **[]*dentry) (*dentry, error) { + rp := resolvingPathParent(vfsRP) + if err := fs.revalidatePath(ctx, rp, d, ds); err != nil { return nil, err } - for !rp.Final() { + for !rp.done() { d.opMu.RLock() next, followedSymlink, err := fs.stepLocked(ctx, rp, d, true /* mayFollowSymlinks */, ds) d.opMu.RUnlock() @@ -357,7 +358,7 @@ func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.Resolving } d = next if followedSymlink { - if err := fs.revalidateParentDir(ctx, rp, d, ds); err != nil { + if err := fs.revalidatePath(ctx, rp, d, ds); err != nil { return nil, err } } @@ -371,12 +372,13 @@ func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.Resolving // resolveLocked resolves rp to an existing file. // // Preconditions: fs.renameMu must be locked. -func (fs *filesystem) resolveLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { +func (fs *filesystem) resolveLocked(ctx context.Context, vfsRP *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { + rp := resolvingPathFull(vfsRP) d := rp.Start().Impl().(*dentry) if err := fs.revalidatePath(ctx, rp, d, ds); err != nil { return nil, err } - for !rp.Done() { + for !rp.done() { d.opMu.RLock() next, followedSymlink, err := fs.stepLocked(ctx, rp, d, true /* mayFollowSymlinks */, ds) d.opMu.RUnlock() @@ -599,7 +601,7 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b return linuxerr.ENOENT } } else { - child, _, err = fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, &ds) + child, _, err = fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, &ds) if err != nil { return err } @@ -872,7 +874,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v // to creating a synthetic one, i.e. one that is kept entirely in memory. // Check that we're not overriding an existing file with a synthetic one. - _, _, err := fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, ds) // +checklocksforce: parent.opMu taken by doCreateAt. + _, _, err := fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, ds) // +checklocksforce: parent.opMu taken by doCreateAt. switch { case err == nil: // Step succeeded, another file exists. @@ -972,7 +974,7 @@ afterTrailingSymlink: // serializing OpenAt calls in the same directory in the common case // that the file exists. parent.opMu.RLock() - child, followedSymlink, err := fs.stepLocked(ctx, rp, parent, true /* mayFollowSymlinks */, &ds) + child, followedSymlink, err := fs.stepLocked(ctx, resolvingPathFull(rp), parent, true /* mayFollowSymlinks */, &ds) parent.opMu.RUnlock() if followedSymlink { if mustCreate { @@ -1012,7 +1014,7 @@ afterTrailingSymlink: // Step to the file again. Since we still hold opMu for // writing, there can't be a race here. - child, _, err = fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, &ds) + child, _, err = fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, &ds) parent.opMu.Unlock() } if err != nil { diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index ea807e86c..61dcc73e3 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -2198,3 +2198,40 @@ func (fd *fileDescription) LockPOSIX(ctx context.Context, uid fslock.UniqueID, o func (fd *fileDescription) UnlockPOSIX(ctx context.Context, uid fslock.UniqueID, r fslock.LockRange) error { return fd.Locks().UnlockPOSIX(ctx, uid, r) } + +// resolvingPath is just a wrapper around *vfs.ResolvingPath. It additionally +// holds some information around the intent behind resolving the path. +type resolvingPath struct { + *vfs.ResolvingPath + + // excludeLast indicates whether the intent is to resolve until the last path + // component. If true, the last path component should remain unresolved. + excludeLast bool +} + +func resolvingPathFull(rp *vfs.ResolvingPath) resolvingPath { + return resolvingPath{ResolvingPath: rp, excludeLast: false} +} + +func resolvingPathParent(rp *vfs.ResolvingPath) resolvingPath { + return resolvingPath{ResolvingPath: rp, excludeLast: true} +} + +func (rp *resolvingPath) done() bool { + if rp.excludeLast { + return rp.Final() + } + return rp.Done() +} + +func (rp *resolvingPath) copy() resolvingPath { + return resolvingPath{ + ResolvingPath: rp.ResolvingPath.Copy(), + excludeLast: rp.excludeLast, + } +} + +// Precondition: !rp.done() && rp.Component() is not "." or "..". +func (rp *resolvingPath) getComponents(emit func(string) bool) { + rp.GetComponents(rp.excludeLast, emit) +} diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index 578f429b2..0b4c2f831 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -286,14 +286,14 @@ func (d *lisafsDentry) getRemoteChild(ctx context.Context, name string) (*dentry // - fs.renameMu must be locked. // - d.opMu must be locked. // - d.isDir(). -// - !rp.Done() && rp.Component() is not "." or "..". +// - !rp.done() && rp.Component() is not "." or "..". // // Postcondition: The returned dentry is already cached appropriately. -func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { +func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp resolvingPath, ds **[]*dentry) (*dentry, error) { // Collect as many path components as possible to walk. var namesArr [16]string // arbitrarily sized array to help avoid slice allocation. names := namesArr[:0] - rp.GetComponents(func(name string) bool { + rp.getComponents(func(name string) bool { if name == "." { return true } diff --git a/pkg/sentry/fsimpl/gofer/revalidate.go b/pkg/sentry/fsimpl/gofer/revalidate.go index 7f0c42a48..2448a3fa2 100644 --- a/pkg/sentry/fsimpl/gofer/revalidate.go +++ b/pkg/sentry/fsimpl/gofer/revalidate.go @@ -44,7 +44,7 @@ func (errRevalidationStepDone) Error() string { // // Preconditions: // - fs.renameMu must be locked. -func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig *vfs.ResolvingPath, start *dentry, ds **[]*dentry) error { +func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig resolvingPath, start *dentry, ds **[]*dentry) error { // Revalidation is done even if start is synthetic in case the path is // something like: ../non_synthetic_file. if fs.opts.interop != InteropModeShared { @@ -52,26 +52,8 @@ func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig *vfs.ResolvingP } // Copy resolving path to walk the path for revalidation. - rp := rpOrig.Copy() - err := fs.revalidate(ctx, rp, start, rp.Done, ds) - rp.Release(ctx) - return err -} - -// revalidateParentDir does the same as revalidatePath, but stops at the parent. -// -// Preconditions: -// - fs.renameMu must be locked. -func (fs *filesystem) revalidateParentDir(ctx context.Context, rpOrig *vfs.ResolvingPath, start *dentry, ds **[]*dentry) error { - // Revalidation is done even if start is synthetic in case the path is - // something like: ../non_synthetic_file and parent is non synthetic. - if fs.opts.interop != InteropModeShared { - return nil - } - - // Copy resolving path to walk the path for revalidation. - rp := rpOrig.Copy() - err := fs.revalidate(ctx, rp, start, rp.Final, ds) + rp := rpOrig.copy() + err := fs.revalidate(ctx, rp, start, ds) rp.Release(ctx) return err } @@ -111,12 +93,12 @@ func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFile // Preconditions: // - fs.renameMu must be locked. // - InteropModeShared is in effect. -func (fs *filesystem) revalidate(ctx context.Context, rp *vfs.ResolvingPath, start *dentry, done func() bool, ds **[]*dentry) error { +func (fs *filesystem) revalidate(ctx context.Context, rp resolvingPath, start *dentry, ds **[]*dentry) error { state := makeRevalidateState(start, true /* refreshStart */) defer state.release() done: - for cur := start; !done(); { + for cur := start; !rp.done(); { var err error cur, err = fs.revalidateStep(ctx, rp, cur, state) if err != nil { @@ -161,7 +143,7 @@ done: // - fs.renameMu must be locked. // - !rp.Done(). // - InteropModeShared is in effect (assumes no negative dentries). -func (fs *filesystem) revalidateStep(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, state *revalidateState) (*dentry, error) { +func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *dentry, state *revalidateState) (*dentry, error) { switch name := rp.Component(); name { case ".": // Do nothing. diff --git a/pkg/sentry/vfs/resolving_path.go b/pkg/sentry/vfs/resolving_path.go index ad059a6f7..e1f9f9150 100644 --- a/pkg/sentry/vfs/resolving_path.go +++ b/pkg/sentry/vfs/resolving_path.go @@ -255,12 +255,16 @@ func (rp *ResolvingPath) Advance() { } // GetComponents emits all the remaining path components in rp. It does *not* -// update rp state. It halts if emit() returns false. -func (rp *ResolvingPath) GetComponents(emit func(string) bool) { +// update rp state. It halts if emit() returns false. If excludeLast is true, +// then the last path component is not emitted. +func (rp *ResolvingPath) GetComponents(excludeLast bool, emit func(string) bool) { // Copy rp state. cur := rp.pit curPart := rp.curPart for cur.Ok() { + if excludeLast && curPart == 0 && !cur.NextOk() { + break + } if !emit(cur.String()) { break }