diff --git a/pkg/sentry/vfs/dentry.go b/pkg/sentry/vfs/dentry.go index 9fcabdcd8..d574162c6 100644 --- a/pkg/sentry/vfs/dentry.go +++ b/pkg/sentry/vfs/dentry.go @@ -337,10 +337,8 @@ func (vfs *VirtualFilesystem) CommitRenameExchangeDentry(from, to *Dentry) { func (vfs *VirtualFilesystem) forgetDeadMountpoint(ctx context.Context, d *Dentry) []refs.RefCounter { vfs.lockMounts() defer vfs.unlockMounts(ctx) - vfs.mounts.seq.BeginWrite() for mnt := range vfs.mountpoints[d] { - vfs.umountRecursiveLocked(mnt, &umountRecursiveOptions{}) + vfs.umountTreeLocked(mnt, &umountRecursiveOptions{}) } - vfs.mounts.seq.EndWrite() return vfs.PopDelayedDecRefs() } diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index 451db54dd..7e73d9ff9 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -594,28 +594,11 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti // TODO(gvisor.dev/issue/1035): Linux special-cases umount of the caller's // root, which we don't implement yet (we'll just fail it since the caller // holds a reference on it). - - propMounts := []*Mount{vd.mount} - if vd.mount.parent() != nil { - for m := nextPropMount(vd.mount.parent(), vd.mount.parent()); m != nil; m = nextPropMount(m, vd.mount.parent()) { - child := vfs.mounts.Lookup(m, vd.mount.point()) - if child == nil { - continue - } - if len(child.children) != 0 && child.coveringMount() == nil { - continue - } - propMounts = append(propMounts, child) - } - } - vfs.mounts.seq.BeginWrite() - for _, m := range propMounts { - vfs.umountRecursiveLocked(m, &umountRecursiveOptions{ - eager: opts.Flags&linux.MNT_DETACH == 0, - disconnectHierarchy: true, - }) - } - vfs.mounts.seq.EndWrite() + vfs.umountTreeLocked(vd.mount, &umountRecursiveOptions{ + eager: opts.Flags&linux.MNT_DETACH == 0, + disconnectHierarchy: true, + propagate: true, + }) return nil } @@ -650,59 +633,81 @@ type umountRecursiveOptions struct { // // disconnectHierarchy is analogous to Linux's !UMOUNT_CONNECTED. disconnectHierarchy bool + + // If propagate is true, mounts located at the same point on the mount's + // parent's peers and follows will also be umounted if they do not have any + // children. + // + // propagate is analogous to Linux's UMOUNT_PROPAGATE. + propagate bool } -// umountRecursiveLocked marks mnt and its descendants as umounted. -// -// umountRecursiveLocked is analogous to Linux's fs/namespace.c:umount_tree(). -// -// Preconditions: -// - vfs.mountMu must be locked. -// - vfs.mounts.seq must be in a writer critical section. +// umountTreeLocked marks mnt and its descendants as umounted. // +// umountTreeLocked is analogous to Linux's fs/namespace.c:umount_tree(). // +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()) - } +func (vfs *VirtualFilesystem) umountTreeLocked(mnt *Mount, opts *umountRecursiveOptions) { + submounts := mnt.submountsLocked() + var umountMnts []*Mount + if opts.disconnectHierarchy { + umountMnts = submounts + } else { + umountMnts = []*Mount{mnt} } - if !mnt.umounted { - mnt.umounted = true + + for _, mnt := range umountMnts { + vfs.umount(mnt) + } + if opts.propagate { + umountMnts = append(umountMnts, vfs.propagateUmount(umountMnts)...) + } + + vfs.mounts.seq.BeginWrite() + for _, mnt := range umountMnts { vfs.delayDecRef(mnt) - if parent := mnt.parent(); parent != nil && (opts.disconnectHierarchy || !parent.umounted) { + if parent := mnt.parent(); parent != nil { vfs.delayDecRef(vfs.disconnectLocked(mnt)) } vfs.setPropagation(mnt, linux.MS_PRIVATE) } + vfs.mounts.seq.EndWrite() + if opts.eager { - for { - refs := mnt.refs.Load() - if refs < 0 { - break - } - if mnt.refs.CompareAndSwap(refs, refs|math.MinInt64) { - break + for _, mnt := range submounts { + for { + refs := mnt.refs.Load() + if refs < 0 { + break + } + if mnt.refs.CompareAndSwap(refs, refs|math.MinInt64) { + break + } } } } - 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) +} + +// +checklocks:vfs.mountMu +func (vfs *VirtualFilesystem) umount(mnt *Mount) { + mnt.umounted = true + if parent := mnt.parent(); parent != nil { + delete(parent.children, mnt) } } +// changeMountpoint disconnects mnt from its current mount point and connects +// it to mp. It must be called from a vfs.mounts.seq writer critical section. +// +// +checklocks:vfs.mountMu +func (vfs *VirtualFilesystem) changeMountpoint(mnt *Mount, mp VirtualDentry) { + mp.dentry.mu.Lock() + vfs.delayDecRef(vfs.disconnectLocked(mnt)) + vfs.delayDecRef(mnt) + mp.IncRef() + vfs.connectLocked(mnt, mp, mp.mount.ns) + mp.dentry.mu.Unlock() +} + // connectLocked makes vd the mount parent/point for mnt. It consumes // references held by vd. // diff --git a/pkg/sentry/vfs/namespace.go b/pkg/sentry/vfs/namespace.go index c74271e52..7b4ee7bff 100644 --- a/pkg/sentry/vfs/namespace.go +++ b/pkg/sentry/vfs/namespace.go @@ -195,11 +195,9 @@ func (vfs *VirtualFilesystem) CloneMountNamespace( func (mntns *MountNamespace) Destroy(ctx context.Context) { vfs := mntns.root.fs.VirtualFilesystem() vfs.lockMounts() - vfs.mounts.seq.BeginWrite() - vfs.umountRecursiveLocked(mntns.root, &umountRecursiveOptions{ + vfs.umountTreeLocked(mntns.root, &umountRecursiveOptions{ disconnectHierarchy: true, }) - vfs.mounts.seq.EndWrite() vfs.unlockMounts(ctx) } diff --git a/pkg/sentry/vfs/propagation.go b/pkg/sentry/vfs/propagation.go index d12783b6c..ec7e64b5c 100644 --- a/pkg/sentry/vfs/propagation.go +++ b/pkg/sentry/vfs/propagation.go @@ -503,6 +503,126 @@ func (vfs *VirtualFilesystem) allocMountGroupIDs(mnt *Mount, recursive bool) err return nil } +// propagateUmount returns a list of mounts that the umount of mnts propagates +// to. +// +// Prerequisites: all the mounts in mnts have had vfs.umount() called on them. +// +// +checklocks:vfs.mountMu +func (vfs *VirtualFilesystem) propagateUmount(mnts []*Mount) []*Mount { + const ( + umountVisited = iota + umountRestore + ) + var toUmount []*Mount + // Processed contains all the mounts that the algorithm has processed so far. + // If the mount maps to umountRestore, it should be restored after processing + // all the mounts. This happens in cases where a mount was speculatively + // unmounted that had children or is a cover mount. + processed := make(map[*Mount]int) + + // Iterate through the mounts from the leafs back to the root. + for i := len(mnts) - 1; i >= 0; i-- { + mnt := mnts[i] + + // If a mount has already been visited we know all its peers and followers + // have been visited so there's no need to visit them again. + if _, ok := processed[mnt]; ok { + continue + } + processed[mnt] = umountVisited + + parent := mnt.parent() + if parent == nil { + continue + } + for m := nextPropMount(parent, parent); m != nil; m = nextPropMount(m, parent) { + child := vfs.mounts.Lookup(m, mnt.point()) + if child == nil { + continue + } + if _, ok := processed[child]; ok { + // If the child has been visited we know its peer group and followers + // have all been visited so there's no need to visit them again. We can + // skip this propagation subtree by setting the iterator to be the last + // mount in the follower group. + if !child.followerList.Empty() { + m = child.followerList.Back() + } + continue + } else if child.umounted { + // If this child has already been marked for unmounting, just mark it + // as visited and move on. This means it was either part of the original + // mount list passed to this method or was umounted from another mount's + // propagation. In either case we can consider all its peers and + // followers as visited. + processed[child] = umountVisited + continue + } + + // This loop starts at the child we are propagating the umount to and + // iterates through the child's parents. It continues as until it + // encounters a parent that's been visited. + loop: + for { + if child.umounted { + break + } + // If there are any children that have mountpoint != parent's root then + // the current mount cannot be unmounted. + for gchild := range child.children { + if gchild.point() == child.root { + continue + } + processed[child] = umountRestore + break loop + } + vfs.umount(child) + toUmount = append(toUmount, child) + child = child.parent() + // If this parent was a mount that had to be restored because it had + // children, it might be safe to umount now that its child is gone. If + // it has been visited then it's already being umounted. + if _, ok := processed[child]; !ok { + break + } + } + } + } + + // Add all the children of mounts marked for umount to the umount list. This + // excludes "cover" mounts (mounts whose mount point is equal to their + // parent's root) which will be reparented in the next step. + for i := 0; i < len(toUmount); i++ { + umount := toUmount[i] + for child := range umount.children { + if child.point() == umount.root { + processed[child] = umountRestore + } else { + vfs.umount(child) + toUmount = append(toUmount, child) + } + } + } + + vfs.mounts.seq.BeginWrite() + for m, status := range processed { + if status == umountVisited { + continue + } + mp := m.getKey() + for mp.mount.umounted { + mp = mp.mount.getKey() + } + if mp != m.getKey() { + vfs.changeMountpoint(m, mp) + } + } + vfs.mounts.seq.EndWrite() + + return toUmount +} + // peerUnderRoot iterates through mnt's peers until it finds a mount that is in // ns and is reachable from root. This method is analogous to // fs/pnode.c:get_peer_under_root() in Linux. diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index f0ce813c4..dd243015c 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -1201,6 +1201,32 @@ TEST(MountTest, PropagateUmountEvent) { } } +TEST(MountTest, PropagateChildUmountEvent) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + TempPath const dir1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + ASSERT_THAT(mount("", dir1.path().c_str(), kTmpfs, 0, ""), SyscallSucceeds()); + + TempPath const child = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir1.path())); + ASSERT_THAT(mount("", child.path().c_str(), kTmpfs, 0, ""), + SyscallSucceeds()); + ASSERT_THAT(mount("", child.path().c_str(), "", MS_SHARED, 0), + SyscallSucceeds()); + + TempPath const dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + ASSERT_THAT(mount(child.path().c_str(), dir2.path().c_str(), "", MS_BIND, 0), + SyscallSucceeds()); + ASSERT_THAT(mount("", child.path().c_str(), kTmpfs, 0, ""), + SyscallSucceeds()); + ASSERT_THAT(umount2(dir1.path().c_str(), MNT_DETACH), SyscallSucceeds()); + + auto optionals = ASSERT_NO_ERRNO_AND_VALUE(MountOptionals()); + EXPECT_EQ(optionals[dir2.path()].size(), 1); + + ASSERT_EQ(umount2(dir2.path().c_str(), MNT_DETACH), 0); +} + TEST(MountTest, UmountIgnoresPeersWithChildren) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); auto const dir1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());