Collect the umount propagation tree before locking the mount table.

Unlocking the mount table while unmounting mounts was racing with other
umount calls.

This change also modifies forgetDeadMountpoints and mount namespace
DecRef to only unmount the specified mounts and not all their peers
as well. Unmounting peers should only happen through calls to
umount.

PiperOrigin-RevId: 487368800
This commit is contained in:
Lucas Manning
2022-11-09 15:53:06 -08:00
committed by gVisor bot
parent 935f29ce6e
commit ead2fa0810
2 changed files with 30 additions and 48 deletions
+1 -3
View File
@@ -336,9 +336,7 @@ func (vfs *VirtualFilesystem) forgetDeadMountpoint(ctx context.Context, d *Dentr
vfs.mountMu.Lock()
vfs.mounts.seq.BeginWrite()
for mnt := range vfs.mountpoints[d] {
vds, mounts := vfs.umountAtRecursiveLocked(ctx, VirtualDentry{mnt, d}, &umountRecursiveOptions{})
vdsToDecRef = append(vdsToDecRef, vds...)
mountsToDecRef = append(mountsToDecRef, mounts...)
vdsToDecRef, mountsToDecRef = vfs.umountRecursiveLocked(mnt, &umountRecursiveOptions{}, vdsToDecRef, mountsToDecRef)
}
vfs.mounts.seq.EndWrite()
vfs.mountMu.Unlock()
+29 -45
View File
@@ -652,6 +652,23 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti
}
}
umountTree := []*Mount{vd.mount}
parent, mountpoint := vd.mount.parent(), vd.mount.point()
if parent.propType == Shared {
for peer := parent.sharedList.Front(); peer != nil; peer = peer.sharedEntry.Next() {
if peer == parent {
continue
}
umountMnt := vfs.mounts.Lookup(peer, mountpoint)
// 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 {
umountTree = append(umountTree, umountMnt)
}
}
}
// 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).
@@ -674,10 +691,16 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti
return linuxerr.EBUSY
}
}
vdsToDecRef, mountsToDecRef := vfs.umountAtRecursiveLocked(ctx, vd, &umountRecursiveOptions{
eager: opts.Flags&linux.MNT_DETACH == 0,
disconnectHierarchy: true,
})
var (
vdsToDecRef []VirtualDentry
mountsToDecRef []*Mount
)
for _, mnt := range umountTree {
vdsToDecRef, mountsToDecRef = vfs.umountRecursiveLocked(mnt, &umountRecursiveOptions{
eager: opts.Flags&linux.MNT_DETACH == 0,
disconnectHierarchy: true,
}, vdsToDecRef, mountsToDecRef)
}
vfs.mounts.seq.EndWrite()
vfs.mountMu.Unlock()
for _, vd := range vdsToDecRef {
@@ -689,45 +712,6 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti
return nil
}
// umountAtRecursiveLocked marks the mount located at vd and its decendendents
// as umounted and does the same for mounts at the same dentry in peers of
// vd.mount's parent.
//
// Preconditions:
// - vd is a mountpoint.
// - vfs.mountMu must be locked.
// - vfs.mounts.seq must be in a writer critical section.
//
// +checklocks:vfs.mountMu
func (vfs *VirtualFilesystem) umountAtRecursiveLocked(ctx context.Context, vd VirtualDentry, opts *umountRecursiveOptions) ([]VirtualDentry, []*Mount) {
parent, mountpoint := vd.mount.parent(), vd.mount.point()
vdsToDecRef, mountsToDecRef := vfs.umountRecursiveLocked(vd.mount, opts, nil, nil)
if parent != nil && parent.propType == Shared {
for peer := parent.sharedList.Front(); peer != nil; peer = peer.sharedEntry.Next() {
if peer == parent {
continue
}
vfs.mounts.seq.EndWrite()
mnt := vfs.mounts.Lookup(peer, mountpoint)
vfs.mounts.seq.BeginWrite()
if mnt == nil {
continue
}
// 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 len(mnt.children) != 0 {
continue
}
vdsToDecRef, mountsToDecRef = vfs.umountRecursiveLocked(mnt, opts, vdsToDecRef, mountsToDecRef)
}
}
return vdsToDecRef, mountsToDecRef
}
// +stateify savable
type umountRecursiveOptions struct {
// If eager is true, ensure that future calls to Mount.tryIncMountedRef()
@@ -934,9 +918,9 @@ func (mntns *MountNamespace) DecRef(ctx context.Context) {
mntns.MountNamespaceRefs.DecRef(func() {
vfs.mountMu.Lock()
vfs.mounts.seq.BeginWrite()
vdsToDecRef, mountsToDecRef := vfs.umountAtRecursiveLocked(ctx, mntns.Root(), &umountRecursiveOptions{
vdsToDecRef, mountsToDecRef := vfs.umountRecursiveLocked(mntns.root, &umountRecursiveOptions{
disconnectHierarchy: true,
})
}, nil, nil)
vfs.mounts.seq.EndWrite()
vfs.mountMu.Unlock()
for _, vd := range vdsToDecRef {