Fix vfs.ConnectMountAt() to handle vfs.mountMu locking correctly.

Fixes 283b80a456 ("Fix logic bug in attaching mounts.")

There were 2 issues introduced in the culprit change:
1. vfs.mountMu was not being unlocked when connectMountAtLocked() failed.
2. The precondition of vfs.abortPropagationTree() was being violated, as it was
   being called with vfs.mountMu unlocked in the ENOSPC case.

Fixed both issues. This also got rid of the need for +checklocksforce
annotation.

PiperOrigin-RevId: 532958291
This commit is contained in:
Ayush Ranjan
2023-05-17 17:17:39 -07:00
committed by gVisor bot
parent a993ef51cd
commit 2936774db6
+7 -13
View File
@@ -23,7 +23,6 @@ import (
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/atomicbitops"
"gvisor.dev/gvisor/pkg/cleanup"
"gvisor.dev/gvisor/pkg/context"
"gvisor.dev/gvisor/pkg/errors/linuxerr"
"gvisor.dev/gvisor/pkg/refs"
@@ -274,12 +273,9 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
}
vfs.mountMu.Lock()
tree := vfs.preparePropagationTree(mnt, vd)
cleanup := cleanup.Make(func() {
vfs.abortPropagationTree(ctx, tree) // +checklocksforce
})
defer cleanup.Clean()
// Check if the new mount + all the propagation mounts puts us over the max.
if uint32(len(tree)+1)+vd.mount.ns.mounts > MountMax {
vfs.abortPropagationTree(ctx, tree)
// We need to unlock mountMu first because DecRef takes a lock on the
// filesystem mutex in some implementations, which can lead to circular
// locking.
@@ -288,11 +284,12 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
return linuxerr.ENOSPC
}
if err := vfs.connectMountAtLocked(ctx, mnt, vd); err != nil {
vfs.abortPropagationTree(ctx, tree)
vfs.mountMu.Unlock()
return err
}
vfs.commitPropagationTree(ctx, tree)
vfs.mountMu.Unlock()
cleanup.Release()
return nil
}
@@ -404,24 +401,21 @@ func (vfs *VirtualFilesystem) BindAt(ctx context.Context, creds *auth.Credential
vfs.mergePeerGroup(sourceVd.mount, clone)
}
}
cleanup := cleanup.Make(func() {
// Checklocks doesn't work with anon functions.
vfs.setPropagation(clone, Private) // +checklocksforce
vfs.abortPropagationTree(ctx, tree) // +checklocksforce
})
defer cleanup.Clean()
if uint32(1+len(tree))+targetVd.mount.ns.mounts > MountMax {
vfs.setPropagation(clone, Private)
vfs.abortPropagationTree(ctx, tree)
vfs.mountMu.Unlock()
targetVd.DecRef(ctx)
return nil, linuxerr.ENOSPC
}
if err := vfs.connectMountAtLocked(ctx, clone, targetVd); err != nil {
vfs.setPropagation(clone, Private)
vfs.abortPropagationTree(ctx, tree)
vfs.mountMu.Unlock()
return nil, err
}
vfs.commitPropagationTree(ctx, tree)
vfs.mountMu.Unlock()
cleanup.Release()
return clone, nil
}