vfs: validate source/target type match in BindAt

pkg/sentry/fsimpl/erofs/erofs.go

@@ -442,6 +442,11 @@ func (d *dentry) Watches() *vfs.Watches {
 // OnZeroWatches implements vfs.DentryImpl.OnZeroWatches.
 func (d *dentry) OnZeroWatches(ctx context.Context) {}
 
+// IsDir implements vfs.DentryImpl.IsDir.
+func (d *dentry) IsDir() bool {
+	return d.inode.IsDir()
+}
+
 func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, opts *vfs.OpenOptions) (*vfs.FileDescription, error) {
 	ats := vfs.AccessTypesForOpenFlags(opts)
 	if err := d.inode.checkPermissions(rp.Credentials(), ats); err != nil {

pkg/sentry/fsimpl/gofer/gofer.go

@@ -1621,6 +1621,11 @@ func (d *dentry) OnZeroWatches(ctx context.Context) {
 	d.checkCachingLocked(ctx, false /* renameMuWriteLocked */)
 }
 
+// IsDir implements vfs.DentryImpl.IsDir.
+func (d *dentry) IsDir() bool {
+	return d.isDir()
+}
+
 // checkCachingLocked should be called after d's reference count becomes 0 or
 // it becomes disowned.
 //

pkg/sentry/fsimpl/kernfs/kernfs.go

@@ -591,6 +591,11 @@ func (d *Dentry) Watches() *vfs.Watches {
 // OnZeroWatches implements vfs.Dentry.OnZeroWatches.
 func (d *Dentry) OnZeroWatches(context.Context) {}
 
+// IsDir implements vfs.DentryImpl.IsDir.
+func (d *Dentry) IsDir() bool {
+	return d.isDir()
+}
+
 // insertChild inserts child into the vfs dentry cache with the given name under
 // this dentry. This does not update the directory inode, so calling this on its
 // own isn't sufficient to insert a child into a directory.

pkg/sentry/fsimpl/overlay/overlay.go

@@ -803,6 +803,11 @@ func (d *dentry) OnZeroWatches(ctx context.Context) {
 	}
 }
 
+// IsDir implements vfs.DentryImpl.IsDir.
+func (d *dentry) IsDir() bool {
+	return d.isDir()
+}
+
 // iterLayers invokes yield on each layer comprising d, from top to bottom. If
 // any call to yield returns false, iterLayer stops iteration.
 func (d *dentry) iterLayers(yield func(vd vfs.VirtualDentry, isUpper bool) bool) {

pkg/sentry/fsimpl/tmpfs/tmpfs.go

@@ -481,6 +481,11 @@ func (d *dentry) Watches() *vfs.Watches {
 // OnZeroWatches implements vfs.Dentry.OnZeroWatches.
 func (d *dentry) OnZeroWatches(context.Context) {}
 
+// IsDir implements vfs.DentryImpl.IsDir.
+func (d *dentry) IsDir() bool {
+	return d.inode.isDir()
+}
+
 // inode represents a filesystem object.
 //
 // +stateify savable

pkg/sentry/vfs/anonfs.go

@@ -334,3 +334,8 @@ func (d *anonDentry) Watches() *Watches {
 
 // OnZeroWatches implements Dentry.OnZeroWatches.
 func (d *anonDentry) OnZeroWatches(context.Context) {}
+
+// IsDir implements DentryImpl.IsDir.
+func (d *anonDentry) IsDir() bool {
+	return false
+}

pkg/sentry/vfs/dentry.go

@@ -138,6 +138,9 @@ type DentryImpl interface {
 	// may acquire inotify locks, so to prevent deadlock, no inotify locks should
 	// be held by the caller.
 	OnZeroWatches(ctx context.Context)
+
+	// IsDir returns true if the file represented by this dentry is a directory.
+	IsDir() bool
 }
 
 // IncRef increments d's reference count.

pkg/sentry/vfs/mount.go

@@ -563,6 +563,12 @@ func (vfs *VirtualFilesystem) BindAt(ctx context.Context, creds *auth.Credential
 		return err
 	}
 
+	// Linux's graft_tree() (fs/namespace.c) returns ENOTDIR if the source and
+	// target have mismatched types (one is a directory, the other is not).
+	if sourceVd.Dentry().Impl().IsDir() != targetVd.Dentry().Impl().IsDir() {
+		return linuxerr.ENOTDIR
+	}
+
 	vfs.lockMounts()
 	defer vfs.unlockMounts(ctx)
 	mp, err := vfs.lockMountpoint(targetVd)

test/syscalls/linux/mount.cc

@@ -478,6 +478,38 @@ TEST(MountTest, BindMountReadonly) {
   EXPECT_EQ(s.st_size, strlen(msg));
 }
 
+// Test that bind mounting a directory onto a regular file fails with ENOTDIR.
+TEST(MountTest, BindMountDirectoryOntoFileFails) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto const source_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
+  auto const target_file =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path()));
+
+  // Bind mounting a directory onto a file should fail with ENOTDIR.
+  EXPECT_THAT(mount(source_dir.path().c_str(), target_file.path().c_str(),
+                    nullptr, MS_BIND, nullptr),
+              SyscallFailsWithErrno(ENOTDIR));
+}
+
+// Test that bind mounting a regular file onto a directory fails with ENOTDIR.
+TEST(MountTest, BindMountFileOntoDirectoryFails) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto const source_file =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path()));
+  auto const target_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
+
+  // Bind mounting a file onto a directory should fail with ENOTDIR.
+  EXPECT_THAT(mount(source_file.path().c_str(), target_dir.path().c_str(),
+                    nullptr, MS_BIND, nullptr),
+              SyscallFailsWithErrno(ENOTDIR));
+}
+
 PosixErrorOr<absl::Time> ATime(absl::string_view file) {
   struct stat s = {};
   if (stat(std::string(file).c_str(), &s) == -1) {