diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index 02cf2c540..9dc0b293f 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -150,6 +150,24 @@ func (mnt *Mount) generateOptionalTags() string { return optional } +// coveringMount returns a mount that completely covers mnt if it exists and nil +// otherwise. A mount that covers another is one that is the only child of its +// parent and whose mountpoint is its parent's root. +func (mnt *Mount) coveringMount() *Mount { + if len(mnt.children) != 1 { + return nil + } + // Get the child from the children map. + var child *Mount + for child = range mnt.children { + break + } + if child.point() != mnt.root { + return nil + } + return child +} + // NewFilesystem creates a new filesystem object not yet associated with any // mounts. It can be installed into the filesystem tree with ConnectMountAt. // Note that only the filesystem-specific mount options from opts are used by @@ -419,7 +437,10 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti // From https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt: // If any peer has some child mounts, then that mount is not unmounted, // but all other mounts are unmounted. - if umountMnt != nil && len(umountMnt.children) == 0 { + if umountMnt == nil { + continue + } + if len(umountMnt.children) == 0 || umountMnt.coveringMount() != nil { umountTree = append(umountTree, umountMnt) } } @@ -483,6 +504,15 @@ type umountRecursiveOptions struct { // // +checklocks:vfs.mountMu func (vfs *VirtualFilesystem) umountRecursiveLocked(mnt *Mount, opts *umountRecursiveOptions) { + // covered mounts are a special case where the grandchild mount is + // reconnected to the parent after the child is disconnected. + var cover *Mount + if parent := mnt.parent(); parent != nil && !parent.umounted { + if cover = mnt.coveringMount(); cover != nil { + vfs.delayDecRef(vfs.disconnectLocked(cover)) + cover.setKey(mnt.getKey()) + } + } if !mnt.umounted { mnt.umounted = true vfs.delayDecRef(mnt) @@ -507,6 +537,14 @@ func (vfs *VirtualFilesystem) umountRecursiveLocked(mnt *Mount, opts *umountRecu for child := range mnt.children { vfs.umountRecursiveLocked(child, opts) } + if cover != nil { + mp := cover.getKey() + mp.IncRef() + mp.dentry.mu.Lock() + vfs.connectLocked(cover, mp, mp.mount.ns) + mp.dentry.mu.Unlock() + vfs.delayDecRef(cover) + } } // connectLocked makes vd the mount parent/point for mnt. It consumes diff --git a/pkg/sentry/vfs/propagation.go b/pkg/sentry/vfs/propagation.go index 1d34d9740..c7f41beaf 100644 --- a/pkg/sentry/vfs/propagation.go +++ b/pkg/sentry/vfs/propagation.go @@ -128,17 +128,27 @@ func (vfs *VirtualFilesystem) preparePropagationTree(mnt *Mount, vd VirtualDentr func (vfs *VirtualFilesystem) commitPropagationTree(ctx context.Context, tree map[*Mount]VirtualDentry) { // The peer mounts should have no way of being dead if we've reached this // point so its safe to connect without checks. - vfs.mounts.seq.BeginWrite() for mnt, vd := range tree { + // If there is already a mount at this (parent, point), disconnect it and + // reconnect it to the new mount once it is connected. vd.dentry.mu.Lock() - // If mnt isn't connected yet, skip connecting during propagation. - if mntns := vd.mount.ns; mntns != nil { - vfs.connectLocked(mnt, vd, mntns) + child := vfs.mounts.Lookup(vd.mount, vd.dentry) + vfs.mounts.seq.BeginWrite() + if child != nil { + vfs.delayDecRef(vfs.disconnectLocked(child)) } - vd.dentry.mu.Unlock() + vfs.connectLocked(mnt, vd, vd.mount.ns) vfs.delayDecRef(mnt) + + if child != nil { + newmp := VirtualDentry{mnt, mnt.root} + newmp.IncRef() + vfs.connectLocked(child, newmp, newmp.mount.ns) + vfs.delayDecRef(child) + } + vfs.mounts.seq.EndWrite() + vd.dentry.mu.Unlock() } - vfs.mounts.seq.EndWrite() } // abortPropagationTree releases any references held by the mounts and diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 145e26ae0..a0d1b3aab 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -899,6 +899,68 @@ TEST(MountTest, MaxMounts) { umount2(parent.path().c_str(), MNT_DETACH); } +TEST(MountTest, PropagateToSameMountpointStacksMounts) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto const dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto const mnt = ASSERT_NO_ERRNO_AND_VALUE(Mount( + dir.path().c_str(), dir.path().c_str(), "", MS_BIND, "", MNT_DETACH)); + auto const child = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path())); + ASSERT_THAT(mount(child.path().c_str(), child.path().c_str(), "", MS_BIND, 0), + SyscallSucceeds()); + ASSERT_THAT(mount("", dir.path().c_str(), "", MS_SHARED, 0), + SyscallSucceeds()); + auto const mnt2 = ASSERT_NO_ERRNO_AND_VALUE(Mount( + dir.path().c_str(), dir2.path().c_str(), "", MS_BIND, "", MNT_DETACH)); + + std::string dir2_child_path = JoinPath(dir2.path(), Basename(child.path())); + ASSERT_THAT( + mount(dir2_child_path.c_str(), dir2_child_path.c_str(), "", MS_BIND, 0), + SyscallSucceeds()); + + // Check that mounts at the child mount point have distinct parents. + std::vector mounts = + ASSERT_NO_ERRNO_AND_VALUE(ProcSelfMountInfoEntries()); + uint64_t parent_id = 0; + for (auto& minfo : mounts) { + if (minfo.mount_point == child.path()) { + if (parent_id == 0) { + parent_id = minfo.parent_id; + } else { + EXPECT_NE(parent_id, minfo.parent_id); + } + } + } +} + +TEST(MountTest, UmountReparentsCoveredMounts) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto const dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto const mnt = ASSERT_NO_ERRNO_AND_VALUE( + Mount("", dir.path().c_str(), "tmpfs", 0, "", MNT_DETACH)); + ASSERT_THAT(mount("", dir.path().c_str(), "", MS_SHARED, 0), + SyscallSucceeds()); + auto const child = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path())); + ASSERT_THAT(mount("", child.path().c_str(), "tmpfs", 0, 0), + SyscallSucceeds()); + auto const mnt2 = ASSERT_NO_ERRNO_AND_VALUE(Mount( + dir.path().c_str(), dir2.path().c_str(), "", MS_BIND, "", MNT_DETACH)); + + std::string dir2_child_path = JoinPath(dir2.path(), Basename(child.path())); + ASSERT_THAT(mount("", dir2_child_path.c_str(), "tmpfs", 0, 0), + SyscallSucceeds()); + + umount2(dir2_child_path.c_str(), MNT_DETACH); + + auto optionals = ASSERT_NO_ERRNO_AND_VALUE(MountOptionals()); + ASSERT_FALSE(optionals[child.path()].empty()); + EXPECT_NE(optionals[child.path()][0].shared, 0); + EXPECT_TRUE(optionals[dir2_child_path].empty()); +} + // Tests that it is possible to make a shared mount. TEST(MountTest, MakeShared) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));