summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/sentry/fsimpl/gofer/filesystem.go82
-rw-r--r--pkg/sentry/fsimpl/kernfs/filesystem.go7
-rw-r--r--pkg/sentry/fsimpl/overlay/filesystem.go17
-rw-r--r--pkg/sentry/fsimpl/tmpfs/filesystem.go9
-rw-r--r--test/syscalls/linux/mkdir.cc33
5 files changed, 91 insertions, 57 deletions
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index df27554d3..91d5dc174 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -407,33 +407,44 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if err != nil {
return err
}
- if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite|vfs.MayExec); err != nil {
+
+ // Order of checks is important. First check if parent directory can be
+ // executed, then check for existence, and lastly check if mount is writable.
+ if err := parent.checkPermissions(rp.Credentials(), vfs.MayExec); err != nil {
return err
}
name := rp.Component()
if name == "." || name == ".." {
return syserror.EEXIST
}
- if len(name) > maxFilenameLen {
- return syserror.ENAMETOOLONG
- }
if parent.isDeleted() {
return syserror.ENOENT
}
+
+ parent.dirMu.Lock()
+ defer parent.dirMu.Unlock()
+
+ child, err := fs.getChildLocked(ctx, rp.VirtualFilesystem(), parent, name, &ds)
+ switch {
+ case err != nil && err != syserror.ENOENT:
+ return err
+ case child != nil:
+ return syserror.EEXIST
+ }
+
mnt := rp.Mount()
if err := mnt.CheckBeginWrite(); err != nil {
return err
}
defer mnt.EndWrite()
- parent.dirMu.Lock()
- defer parent.dirMu.Unlock()
+
+ if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite); err != nil {
+ return err
+ }
+ if !dir && rp.MustBeDir() {
+ return syserror.ENOENT
+ }
if parent.isSynthetic() {
- if child := parent.children[name]; child != nil {
- return syserror.EEXIST
- }
- if !dir && rp.MustBeDir() {
- return syserror.ENOENT
- }
if createInSyntheticDir == nil {
return syserror.EPERM
}
@@ -449,47 +460,20 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
parent.watches.Notify(ctx, name, uint32(ev), 0, vfs.InodeEvent, false /* unlinked */)
return nil
}
- if fs.opts.interop == InteropModeShared {
- 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.
- // Just attempt the file creation RPC instead. If a file does exist, the
- // RPC will fail with EEXIST like we would have. If the RPC succeeds, and a
- // stale dentry exists, the dentry will fail revalidation next time it's
- // used.
- if err := createInRemoteDir(parent, name, &ds); err != nil {
- return err
- }
- ev := linux.IN_CREATE
- if dir {
- ev |= linux.IN_ISDIR
- }
- parent.watches.Notify(ctx, name, uint32(ev), 0, vfs.InodeEvent, false /* unlinked */)
- return nil
- }
- 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.
+ // 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.
if err := createInRemoteDir(parent, name, &ds); err != nil {
return err
}
- if child, ok := parent.children[name]; ok && child == nil {
- // Delete the now-stale negative dentry.
- delete(parent.children, name)
+ if fs.opts.interop != InteropModeShared {
+ if child, ok := parent.children[name]; ok && child == nil {
+ // Delete the now-stale negative dentry.
+ delete(parent.children, name)
+ }
+ parent.touchCMtime()
+ parent.dirents = nil
}
- parent.touchCMtime()
- parent.dirents = nil
ev := linux.IN_CREATE
if dir {
ev |= linux.IN_ISDIR
diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go
index e77523f22..a7a553619 100644
--- a/pkg/sentry/fsimpl/kernfs/filesystem.go
+++ b/pkg/sentry/fsimpl/kernfs/filesystem.go
@@ -208,7 +208,9 @@ func (fs *Filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.Resolving
// * Filesystem.mu must be locked for at least reading.
// * isDir(parentInode) == true.
func checkCreateLocked(ctx context.Context, creds *auth.Credentials, name string, parent *Dentry) error {
- if err := parent.inode.CheckPermissions(ctx, creds, vfs.MayWrite|vfs.MayExec); err != nil {
+ // Order of checks is important. First check if parent directory can be
+ // executed, then check for existence, and lastly check if mount is writable.
+ if err := parent.inode.CheckPermissions(ctx, creds, vfs.MayExec); err != nil {
return err
}
if name == "." || name == ".." {
@@ -223,6 +225,9 @@ func checkCreateLocked(ctx context.Context, creds *auth.Credentials, name string
if parent.VFSDentry().IsDead() {
return syserror.ENOENT
}
+ if err := parent.inode.CheckPermissions(ctx, creds, vfs.MayWrite); err != nil {
+ return err
+ }
return nil
}
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go
index d55bdc97f..e46f593c7 100644
--- a/pkg/sentry/fsimpl/overlay/filesystem.go
+++ b/pkg/sentry/fsimpl/overlay/filesystem.go
@@ -480,9 +480,6 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if err != nil {
return err
}
- if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite|vfs.MayExec); err != nil {
- return err
- }
name := rp.Component()
if name == "." || name == ".." {
return syserror.EEXIST
@@ -490,11 +487,11 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if parent.vfsd.IsDead() {
return syserror.ENOENT
}
- mnt := rp.Mount()
- if err := mnt.CheckBeginWrite(); err != nil {
+
+ if err := parent.checkPermissions(rp.Credentials(), vfs.MayExec); err != nil {
return err
}
- defer mnt.EndWrite()
+
parent.dirMu.Lock()
defer parent.dirMu.Unlock()
@@ -514,6 +511,14 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
return syserror.ENOENT
}
+ mnt := rp.Mount()
+ if err := mnt.CheckBeginWrite(); err != nil {
+ return err
+ }
+ defer mnt.EndWrite()
+ if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite|vfs.MayExec); err != nil {
+ return err
+ }
// 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/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go
index 9296db2fb..453e41d11 100644
--- a/pkg/sentry/fsimpl/tmpfs/filesystem.go
+++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go
@@ -153,7 +153,10 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if err != nil {
return err
}
- if err := parentDir.inode.checkPermissions(rp.Credentials(), vfs.MayWrite|vfs.MayExec); err != nil {
+
+ // Order of checks is important. First check if parent directory can be
+ // executed, then check for existence, and lastly check if mount is writable.
+ if err := parentDir.inode.checkPermissions(rp.Credentials(), vfs.MayExec); err != nil {
return err
}
name := rp.Component()
@@ -179,6 +182,10 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
return err
}
defer mnt.EndWrite()
+
+ if err := parentDir.inode.checkPermissions(rp.Credentials(), vfs.MayWrite); err != nil {
+ return err
+ }
if err := create(parentDir, name); err != nil {
return err
}
diff --git a/test/syscalls/linux/mkdir.cc b/test/syscalls/linux/mkdir.cc
index 27758203d..11fbfa5c5 100644
--- a/test/syscalls/linux/mkdir.cc
+++ b/test/syscalls/linux/mkdir.cc
@@ -82,6 +82,39 @@ TEST_F(MkdirTest, FailsOnDirWithoutWritePerms) {
SyscallFailsWithErrno(EACCES));
}
+TEST_F(MkdirTest, DirAlreadyExists) {
+ // Drop capabilities that allow us to override file and directory permissions.
+ ASSERT_NO_ERRNO(SetCapability(CAP_DAC_OVERRIDE, false));
+ ASSERT_NO_ERRNO(SetCapability(CAP_DAC_READ_SEARCH, false));
+
+ ASSERT_THAT(mkdir(dirname_.c_str(), 0777), SyscallSucceeds());
+ auto dir = JoinPath(dirname_.c_str(), "foo");
+ EXPECT_THAT(mkdir(dir.c_str(), 0777), SyscallSucceeds());
+
+ struct {
+ int mode;
+ int err;
+ } tests[] = {
+ {.mode = 0000, .err = EACCES}, // No perm
+ {.mode = 0100, .err = EEXIST}, // Exec only
+ {.mode = 0200, .err = EACCES}, // Write only
+ {.mode = 0300, .err = EEXIST}, // Write+exec
+ {.mode = 0400, .err = EACCES}, // Read only
+ {.mode = 0500, .err = EEXIST}, // Read+exec
+ {.mode = 0600, .err = EACCES}, // Read+write
+ {.mode = 0700, .err = EEXIST}, // All
+ };
+ for (const auto& t : tests) {
+ printf("mode: 0%o\n", t.mode);
+ EXPECT_THAT(chmod(dirname_.c_str(), t.mode), SyscallSucceeds());
+ EXPECT_THAT(mkdir(dir.c_str(), 0777), SyscallFailsWithErrno(t.err));
+ }
+
+ // Clean up.
+ EXPECT_THAT(chmod(dirname_.c_str(), 0777), SyscallSucceeds());
+ ASSERT_THAT(rmdir(dir.c_str()), SyscallSucceeds());
+}
+
TEST_F(MkdirTest, MkdirAtEmptyPath) {
ASSERT_THAT(mkdir(dirname_.c_str(), 0777), SyscallSucceeds());
auto fd =
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-23 19:25:47 +0000