Skip to content

Commit e712060

Browse files
committed
vfs: check umounted in CloneTreeToAnonNS
CloneTreeToAnonNS validated namespace membership but did not check Mount.umounted. After umount2(MNT_DETACH), a mount's .umounted flag is set but .ns is not cleared until the last reference is dropped, so a detached-but-referenced mount passed the existing check and could be cloned via open_tree(OPEN_TREE_CLONE), then re-attached via move_mount(2) -- reversing the detach. This mirrors the fix in 6a112c6 for BindAt, ConnectMountAt, MoveMountAt, and propagateMount. Fixes #13481
1 parent 2cb8057 commit e712060

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

pkg/sentry/vfs/namespace.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,13 @@ func (vfs *VirtualFilesystem) CloneTreeToAnonNS(
257257
// MS_UNBINDABLE mounts should be rejected here.
258258

259259
fsName := fromMnt.Filesystem().FilesystemType().Name()
260+
// fromMnt must not have been detached from its mount tree (e.g. via
261+
// umount(MNT_DETACH)). Note that fromMnt.ns is not cleared on detach,
262+
// only fromMnt.umounted is set, so this must be checked independently
263+
// of the namespace-membership check below.
264+
if fromMnt.umounted {
265+
return nil, linuxerr.EINVAL
266+
}
260267
// fromMnt must be either:
261268
// - In the same mount ns as the current task
262269
// - In an appropriate anonymous mount namespace

test/syscalls/linux/mount_fd.cc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,35 @@ TEST(OpenTreeTest, OpenTreeCloneNonRecursiveOnAttached) {
730730
SyscallFailsWithErrno(ENOENT));
731731
}
732732

733+
TEST(OpenTreeTest, OpenTreeCloneFailsOnDetachedMount) {
734+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
735+
736+
// Create a detached tmpfs mount fd via fsopen/fsmount (no path attachment).
737+
int fsfd = fsopen(kTmpfs, 0);
738+
ASSERT_THAT(fsfd, SyscallSucceeds());
739+
auto cleanup_fs = Cleanup([&]() { close(fsfd); });
740+
ASSERT_THAT(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0),
741+
SyscallSucceeds());
742+
int mntfd = fsmount(fsfd, 0, 0);
743+
ASSERT_THAT(mntfd, SyscallSucceeds());
744+
auto cleanup_mnt = Cleanup([&]() { close(mntfd); });
745+
746+
// Attach the mount to a path and then detach it via MNT_DETACH while
747+
// keeping the mount fd alive.
748+
const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
749+
ASSERT_THAT(
750+
move_mount(mntfd, "", AT_FDCWD, dir.path().c_str(),
751+
MOVE_MOUNT_F_EMPTY_PATH),
752+
SyscallSucceeds());
753+
ASSERT_THAT(umount2(dir.path().c_str(), MNT_DETACH), SyscallSucceeds());
754+
755+
// open_tree(OPEN_TREE_CLONE) on a mount that has already been detached via
756+
// MNT_DETACH must fail with EINVAL, matching Linux. Regression test for
757+
// CloneTreeToAnonNS() not checking Mount.umounted (gvisor.dev/issue/13481).
758+
EXPECT_THAT(open_tree(mntfd, "", AT_EMPTY_PATH | OPEN_TREE_CLONE),
759+
SyscallFailsWithErrno(EINVAL));
760+
}
761+
733762
TEST(OpenTreeTest, OpenTreeCloneNonRecursiveOnDetached) {
734763
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
735764

0 commit comments

Comments
 (0)