diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 745d3d4652b..89f69b91a6a 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -1002,6 +1002,11 @@ func (fs *filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf if mustCreate { return nil, linuxerr.EEXIST } + // The start is a file if the path ends at the root of a file bind + // mount or at a magic link to a file. + if rp.MustBeDir() && !start.isDir() { + return nil, linuxerr.ENOTDIR + } if !start.inode.cachedMetadataAuthoritative() { // Refresh dentry's attributes before opening. if err := start.inode.updateMetadata(ctx); err != nil { diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 9ca8a070388..490df69ef5a 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -841,6 +841,11 @@ func (fs *filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf if mustCreate { return nil, linuxerr.EEXIST } + // The start is a file if the path ends at the root of a file bind + // mount or at a magic link to a file. + if rp.MustBeDir() && !start.isDir() { + return nil, linuxerr.ENOTDIR + } // ensureOpenableLocked may release renameMu, so pin the dentry first. start.IncRef() err := start.ensureOpenableLocked(ctx, rp, &opts) diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 469a49896c2..7cc5627a7b3 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -606,6 +606,23 @@ TEST(MountTest, ReadOnlyBindMountAllowsWriteOpenOfCharDevice) { SyscallSucceedsWithValue(sizeof(msg))); } +// A bind mount of a file has a file as its root, which O_DIRECTORY and a +// trailing slash refuse like any other file. +TEST(MountTest, OpenDirectoryOnFileBindMount) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + auto const file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + auto const target = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + auto const mnt = ASSERT_NO_ERRNO_AND_VALUE( + Mount(file.path(), target.path(), "", MS_BIND, "", 0)); + + EXPECT_THAT(open(target.path().c_str(), O_RDONLY | O_DIRECTORY), + SyscallFailsWithErrno(ENOTDIR)); + EXPECT_THAT(open((target.path() + "/").c_str(), O_RDONLY), + SyscallFailsWithErrno(ENOTDIR)); + EXPECT_NO_ERRNO(Open(target.path(), O_RDONLY)); +} + TEST(MountTest, ReadOnlyBindMountAllowsWriteOpenOfFIFO) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); diff --git a/test/syscalls/linux/open.cc b/test/syscalls/linux/open.cc index 96ec0d82216..c2976b5d467 100644 --- a/test/syscalls/linux/open.cc +++ b/test/syscalls/linux/open.cc @@ -425,6 +425,20 @@ TEST_F(OpenTest, FileNotDirectory) { SyscallFailsWithErrno(ENOTDIR)); } +// A magic link to a file is refused by O_DIRECTORY and a trailing slash like +// the file itself. +TEST_F(OpenTest, MagicLinkToFileNotDirectory) { + auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + const FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDONLY)); + const std::string link = "/proc/self/fd/" + std::to_string(fd.get()); + EXPECT_THAT(open(link.c_str(), O_RDONLY | O_DIRECTORY), + SyscallFailsWithErrno(ENOTDIR)); + EXPECT_THAT(open((link + "/").c_str(), O_RDONLY), + SyscallFailsWithErrno(ENOTDIR)); + EXPECT_NO_ERRNO(Open(link, O_RDONLY)); +} + TEST_F(OpenTest, SymlinkDirectory) { auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); std::string link = NewTempAbsPath();