[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

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())