gofer, overlay: Fail directory opens of bind-mounted files and magic links

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 {

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)

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

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