From fe9442d3270d14c095932d917e4e53e706866217 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Fri, 6 Nov 2020 18:50:33 -0800 Subject: [vfs] Return EEXIST when file already exists and rp.MustBeDir() is true. This is consistent with what Linux does. This was causing a PHP runtime test failure. Fixed it for VFS2. PiperOrigin-RevId: 341155209 --- pkg/sentry/fsimpl/gofer/filesystem.go | 12 +++++++++--- pkg/sentry/fsimpl/kernfs/filesystem.go | 9 +++++++++ pkg/sentry/fsimpl/overlay/filesystem.go | 7 ++++--- test/runtimes/exclude/php7.3.6.csv | 2 +- test/runtimes/runner/lib/lib.go | 5 ++++- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index e327f5fbe..bbb01148b 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -371,9 +371,6 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if len(name) > maxFilenameLen { return syserror.ENAMETOOLONG } - if !dir && rp.MustBeDir() { - return syserror.ENOENT - } if parent.isDeleted() { return syserror.ENOENT } @@ -388,6 +385,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if child := parent.children[name]; child != nil { return syserror.EEXIST } + if !dir && rp.MustBeDir() { + return syserror.ENOENT + } if createInSyntheticDir == nil { return syserror.EPERM } @@ -407,6 +407,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if child := parent.children[name]; child != nil && child.isSynthetic() { return syserror.EEXIST } + if !dir && rp.MustBeDir() { + return syserror.ENOENT + } // The existence of a non-synthetic dentry at name would be inconclusive // because the file it represents may have been deleted from the remote // filesystem, so we would need to make an RPC to revalidate the dentry. @@ -427,6 +430,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if child := parent.children[name]; child != nil { return syserror.EEXIST } + if !dir && rp.MustBeDir() { + return syserror.ENOENT + } // No cached dentry exists; however, there might still be an existing file // at name. As above, we attempt the file creation RPC anyway. if err := createInRemoteDir(parent, name, &ds); err != nil { diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index d125435d9..e77523f22 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -355,6 +355,9 @@ func (fs *Filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. if err := checkCreateLocked(ctx, rp.Credentials(), pc, parent); err != nil { return err } + if rp.MustBeDir() { + return syserror.ENOENT + } if rp.Mount() != vd.Mount() { return syserror.EXDEV } @@ -433,6 +436,9 @@ func (fs *Filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v if err := checkCreateLocked(ctx, rp.Credentials(), pc, parent); err != nil { return err } + if rp.MustBeDir() { + return syserror.ENOENT + } if err := rp.Mount().CheckBeginWrite(); err != nil { return err } @@ -825,6 +831,9 @@ func (fs *Filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, targ if err := checkCreateLocked(ctx, rp.Credentials(), pc, parent); err != nil { return err } + if rp.MustBeDir() { + return syserror.ENOENT + } if err := rp.Mount().CheckBeginWrite(); err != nil { return err } diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 30601e76d..bc07d72c0 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -477,9 +477,6 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if name == "." || name == ".." { return syserror.EEXIST } - if !dir && rp.MustBeDir() { - return syserror.ENOENT - } if parent.vfsd.IsDead() { return syserror.ENOENT } @@ -503,6 +500,10 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir return syserror.EEXIST } + if !dir && rp.MustBeDir() { + return syserror.ENOENT + } + // Ensure that the parent directory is copied-up so that we can create the // new file in the upper layer. if err := parent.copyUpLocked(ctx); err != nil { diff --git a/test/runtimes/exclude/php7.3.6.csv b/test/runtimes/exclude/php7.3.6.csv index 0e8fc2c98..c051fe571 100644 --- a/test/runtimes/exclude/php7.3.6.csv +++ b/test/runtimes/exclude/php7.3.6.csv @@ -30,7 +30,7 @@ ext/standard/tests/file/php_fd_wrapper_04.phpt,, ext/standard/tests/file/realpath_bug77484.phpt,b/162894969,VFS1 only failure ext/standard/tests/file/rename_variation.phpt,b/68717309, ext/standard/tests/file/symlink_link_linkinfo_is_link_variation4.phpt,b/162895341, -ext/standard/tests/file/symlink_link_linkinfo_is_link_variation8.phpt,b/162896223, +ext/standard/tests/file/symlink_link_linkinfo_is_link_variation8.phpt,b/162896223,VFS1 only failure ext/standard/tests/general_functions/escapeshellarg_bug71270.phpt,, ext/standard/tests/general_functions/escapeshellcmd_bug71270.phpt,, ext/standard/tests/streams/proc_open_bug60120.phpt,,Flaky until php-src 3852a35fdbcb diff --git a/test/runtimes/runner/lib/lib.go b/test/runtimes/runner/lib/lib.go index 41616e5e0..64e6e14db 100644 --- a/test/runtimes/runner/lib/lib.go +++ b/test/runtimes/runner/lib/lib.go @@ -40,7 +40,10 @@ func RunTests(lang, image, excludeFile string, partitionNum, totalPartitions, ba return 1 } - // Get tests to exclude.. + // TODO(gvisor.dev/issue/1624): Remove those tests from all exclude lists + // that only fail with VFS1. + + // Get tests to exclude. excludes, err := getExcludes(excludeFile) if err != nil { fmt.Fprintf(os.Stderr, "Error getting exclude list: %s\n", err.Error()) -- cgit v1.2.3