From 1542a9de83adf47971d5f54d7366dc6148d0e24a Mon Sep 17 00:00:00 2001 From: Eric Daniels Date: Wed, 11 Nov 2026 19:36:29 -0800 Subject: [PATCH] gofer, overlay: Fail directory opens of bind-mounted files and magic links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Linux fails open(2) with ENOTDIR when the path ends at a file and has O_DIRECTORY or a trailing slash. Gofer and overlay skipped that check when the walk ended at its starting dentry, which happens at the root of a file bind mount and after a magic link like /proc/self/fd/N, so those opens succeeded. OpenAt now checks MustBeDir there too, as tmpfs, kernfs and erofs already do. I don't know of anything that hits this in practice, but container runtimes bind mount files like /etc/hostname into every container. Tested: the new mount and open tests fail on runsc without the fix and pass with it on native, ptrace, systrap, systrap_directfs and systrap_overlay, and make lint passes. Auto-fixed-by: [🤖] @EtiennePerot FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/gvisor/pull/17249 from edaniels:gofer-overlay-mount-root-mustbedir 61027b3173b99b5305c3c5114fa35603fb3c5bed PiperOrigin-RevId: 1019703700 --- pkg/sentry/fsimpl/gofer/filesystem.go | 5 +++++ pkg/sentry/fsimpl/overlay/filesystem.go | 5 +++++ test/syscalls/linux/mount.cc | 17 +++++++++++++++++ test/syscalls/linux/open.cc | 14 ++++++++++++++ 4 files changed, 41 insertions(+) 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();