netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Drop MappingIdentity ref outside mm.mappingMu critical section.

Browse Source 
Otherwise we get circular locking with various filesystem locks if the
dropped ref is the final ref. Mapping identities are typically file
descriptions on various filesystems, which in turn call mm functions
under the respective filesystem locks.

PiperOrigin-RevId: 442919016
This commit is contained in:
Rahat Mahmood
2022-04-19 14:55:24 -07:00
committed by gVisor bot
parent 86c030ebcd
commit f4e537843f
4 changed files with 128 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/mm/lifecycle.go
+17 -3
View File
@@ -61,6 +61,15 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
defer mm.AddressSpace().PostFork()
mm.metadataMu.Lock()
defer mm.metadataMu.Unlock()
var droppedIDs []memmap.MappingIdentity
// This must run after {mm,mm2}.mappingMu.Unlock().
defer func() {
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}()
mm.mappingMu.RLock()
defer mm.mappingMu.RUnlock()
mm2 := &MemoryManager{
@@ -109,7 +118,7 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
// Inform the Mappable, if any, of the new mapping.
if vma.mappable != nil {
if err := vma.mappable.AddMapping(ctx, mm2, vmaAR, vma.off, vma.canWriteMappableLocked()); err != nil {
mm2.removeVMAsLocked(ctx, mm2.applicationAddrRange())
_, droppedIDs = mm2.removeVMAsLocked(ctx, mm2.applicationAddrRange(), droppedIDs)
return nil, err
}
}
@@ -275,11 +284,16 @@ func (mm *MemoryManager) DecUsers(ctx context.Context) {
}
mm.activeMu.Unlock()
var droppedIDs []memmap.MappingIdentity
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
// If mm is being dropped before mm.SetMmapLayout was called,
// mm.applicationAddrRange() will be empty.
if ar := mm.applicationAddrRange(); ar.Length() != 0 {
mm.unmapLocked(ctx, ar)
_, droppedIDs = mm.unmapLocked(ctx, ar, droppedIDs)
}
mm.mappingMu.Unlock()
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}
pkg/sentry/mm/shm.go
+15 -2
View File
@@ -19,6 +19,7 @@ import (
"gvisor.dev/gvisor/pkg/errors/linuxerr"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/sentry/kernel/shm"
"gvisor.dev/gvisor/pkg/sentry/memmap"
)
// DetachShm unmaps a sysv shared memory segment.
@@ -29,6 +30,16 @@ func (mm *MemoryManager) DetachShm(ctx context.Context, addr hostarch.Addr) erro
}
var detached *shm.Shm
var vgap vmaGapIterator
var droppedIDs []memmap.MappingIdentity
// This must run after mm.mappingMu.Unlock().
defer func() {
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}()
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
@@ -39,7 +50,8 @@ func (mm *MemoryManager) DetachShm(ctx context.Context, addr hostarch.Addr) erro
vma := vseg.ValuePtr()
if shm, ok := vma.mappable.(*shm.Shm); ok && vseg.Start() >= addr && uint64(vseg.Start()-addr) == vma.off {
detached = shm
vseg = mm.unmapLocked(ctx, vseg.Range()).NextSegment()
vgap, droppedIDs = mm.unmapLocked(ctx, vseg.Range(), droppedIDs)
vseg = vgap.NextSegment()
break
} else {
vseg = vseg.NextSegment()
@@ -56,7 +68,8 @@ func (mm *MemoryManager) DetachShm(ctx context.Context, addr hostarch.Addr) erro
for vseg.Ok() && vseg.End() <= end {
vma := vseg.ValuePtr()
if vma.mappable == detached && uint64(vseg.Start()-addr) == vma.off {
vseg = mm.unmapLocked(ctx, vseg.Range()).NextSegment()
vgap, droppedIDs = mm.unmapLocked(ctx, vseg.Range(), droppedIDs)
vseg = vgap.NextSegment()
} else {
vseg = vseg.NextSegment()
}
pkg/sentry/mm/syscalls.go
+60 -16
View File
@@ -115,11 +115,12 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar
}
// Get the new vma.
var droppedIDs []memmap.MappingIdentity
mm.mappingMu.Lock()
if opts.MLockMode < mm.defMLockMode {
opts.MLockMode = mm.defMLockMode
}
vseg, ar, err := mm.createVMALocked(ctx, opts)
vseg, ar, droppedIDs, err := mm.createVMALocked(ctx, opts, droppedIDs)
if err != nil {
mm.mappingMu.Unlock()
return 0, err
@@ -148,6 +149,10 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar
mm.mappingMu.Unlock()
}
for _, id := range droppedIDs {
id.DecRef(ctx)
}
return ar.Start, nil
}
@@ -264,9 +269,11 @@ func (mm *MemoryManager) MapStack(ctx context.Context) (hostarch.AddrRange, erro
return hostarch.AddrRange{}, linuxerr.ENOMEM
}
stackStart := stackEnd - szaddr
var droppedIDs []memmap.MappingIdentity
var ar hostarch.AddrRange
var err error
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
_, ar, err := mm.createVMALocked(ctx, memmap.MMapOpts{
_, ar, droppedIDs, err = mm.createVMALocked(ctx, memmap.MMapOpts{
Length: sz,
Addr: stackStart,
Perms: hostarch.ReadWrite,
@@ -275,7 +282,11 @@ func (mm *MemoryManager) MapStack(ctx context.Context) (hostarch.AddrRange, erro
GrowsDown: true,
MLockMode: mm.defMLockMode,
Hint: "[stack]",
})
}, droppedIDs)
mm.mappingMu.Unlock()
for _, id := range droppedIDs {
id.DecRef(ctx)
}
return ar, err
}
@@ -296,9 +307,15 @@ func (mm *MemoryManager) MUnmap(ctx context.Context, addr hostarch.Addr, length
return linuxerr.EINVAL
}
var droppedIDs []memmap.MappingIdentity
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
mm.unmapLocked(ctx, ar)
_, droppedIDs = mm.unmapLocked(ctx, ar, droppedIDs)
mm.mappingMu.Unlock()
for _, id := range droppedIDs {
id.DecRef(ctx)
}
return nil
}
@@ -350,6 +367,14 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
return 0, linuxerr.EINVAL
}
var droppedIDs []memmap.MappingIdentity
// This must run after mm.mappingMu.Unlock().
defer func() {
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}()
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
@@ -394,7 +419,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
// If oldAddr+oldSize didn't overflow, oldAddr+newSize can't
// either.
newEnd := oldAddr + hostarch.Addr(newSize)
mm.unmapLocked(ctx, hostarch.AddrRange{newEnd, oldEnd})
_, droppedIDs = mm.unmapLocked(ctx, hostarch.AddrRange{newEnd, oldEnd}, droppedIDs)
}
return oldAddr, nil
}
@@ -411,7 +436,10 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
if vma.mappable != nil {
newOffset = vseg.mappableRange().End
}
vseg, ar, err := mm.createVMALocked(ctx, memmap.MMapOpts{
var vseg vmaIterator
var ar hostarch.AddrRange
var err error
vseg, ar, droppedIDs, err = mm.createVMALocked(ctx, memmap.MMapOpts{
Length: newSize - oldSize,
MappingIdentity: vma.id,
Mappable: vma.mappable,
@@ -424,7 +452,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
GrowsDown: vma.growsDown,
MLockMode: vma.mlockMode,
Hint: vma.hint,
})
}, droppedIDs)
if err == nil {
if vma.mlockMode == memmap.MLockEager {
mm.populateVMA(ctx, vseg, ar, true)
@@ -473,7 +501,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
}
// Unmap any mappings at the destination.
mm.unmapLocked(ctx, newAR)
_, droppedIDs = mm.unmapLocked(ctx, newAR, droppedIDs)
// If the sizes specify shrinking, unmap everything between the new and
// old sizes at the source. Unmapping before the following checks is
@@ -481,7 +509,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS
// vma_to_resize().
if newSize < oldSize {
oldNewEnd := oldAddr + hostarch.Addr(newSize)
mm.unmapLocked(ctx, hostarch.AddrRange{oldNewEnd, oldEnd})
_, droppedIDs = mm.unmapLocked(ctx, hostarch.AddrRange{oldNewEnd, oldEnd}, droppedIDs)
oldEnd = oldNewEnd
}
@@ -690,13 +718,17 @@ func (mm *MemoryManager) MProtect(addr hostarch.Addr, length uint64, realPerms h
// BrkSetup sets mm's brk address to addr and its brk size to 0.
func (mm *MemoryManager) BrkSetup(ctx context.Context, addr hostarch.Addr) {
var droppedIDs []memmap.MappingIdentity
mm.mappingMu.Lock()
defer mm.mappingMu.Unlock()
// Unmap the existing brk.
if mm.brk.Length() != 0 {
mm.unmapLocked(ctx, mm.brk)
_, droppedIDs = mm.unmapLocked(ctx, mm.brk, droppedIDs)
}
mm.brk = hostarch.AddrRange{addr, addr}
mm.mappingMu.Unlock()
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}
// Brk implements the semantics of Linux's brk(2), except that it returns an
@@ -730,9 +762,21 @@ func (mm *MemoryManager) Brk(ctx context.Context, addr hostarch.Addr) (hostarch.
return addr, linuxerr.EFAULT
}
var vseg vmaIterator
var ar hostarch.AddrRange
var err error
var droppedIDs []memmap.MappingIdentity
// This must run after mm.mappingMu.Unlock().
defer func() {
for _, id := range droppedIDs {
id.DecRef(ctx)
}
}()
switch {
case oldbrkpg < newbrkpg:
vseg, ar, err := mm.createVMALocked(ctx, memmap.MMapOpts{
vseg, ar, droppedIDs, err = mm.createVMALocked(ctx, memmap.MMapOpts{
Length: uint64(newbrkpg - oldbrkpg),
Addr: oldbrkpg,
Fixed: true,
@@ -745,7 +789,7 @@ func (mm *MemoryManager) Brk(ctx context.Context, addr hostarch.Addr) (hostarch.
// mm->def_flags.
MLockMode: mm.defMLockMode,
Hint: "[heap]",
})
}, droppedIDs)
if err != nil {
addr = mm.brk.End
mm.mappingMu.Unlock()
@@ -759,7 +803,7 @@ func (mm *MemoryManager) Brk(ctx context.Context, addr hostarch.Addr) (hostarch.
}
case newbrkpg < oldbrkpg:
mm.unmapLocked(ctx, hostarch.AddrRange{newbrkpg, oldbrkpg})
_, droppedIDs = mm.unmapLocked(ctx, hostarch.AddrRange{newbrkpg, oldbrkpg}, droppedIDs)
fallthrough
default:
pkg/sentry/mm/vma.go
+36 -17
View File
@@ -28,10 +28,16 @@ import (
"gvisor.dev/gvisor/pkg/sentry/memmap"
)
// Caller provides the droppedIDs slice to collect dropped mapping
// identities. The caller must drop the references on these identities outside a
// mm.mappingMu critical section. droppedIDs has append-like semantics, multiple
// calls to functions that drop mapping identities within a scope should reuse
// the same slice.
//
// Preconditions:
// * mm.mappingMu must be locked for writing.
// * opts must be valid as defined by the checks in MMap.
func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOpts) (vmaIterator, hostarch.AddrRange, error) {
func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOpts, droppedIDs []memmap.MappingIdentity) (vmaIterator, hostarch.AddrRange, []memmap.MappingIdentity, error) {
if opts.MaxPerms != opts.MaxPerms.Effective() {
panic(fmt.Sprintf("Non-effective MaxPerms %s cannot be enforced", opts.MaxPerms))
}
@@ -48,7 +54,7 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
if opts.Force && opts.Unmap && opts.Fixed {
addr = opts.Addr
} else {
return vmaIterator{}, hostarch.AddrRange{}, err
return vmaIterator{}, hostarch.AddrRange{}, droppedIDs, err
}
}
ar, _ := addr.ToRange(opts.Length)
@@ -59,7 +65,7 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
newUsageAS -= uint64(mm.vmas.SpanRange(ar))
}
if limitAS := limits.FromContext(ctx).Get(limits.AS).Cur; newUsageAS > limitAS {
return vmaIterator{}, hostarch.AddrRange{}, linuxerr.ENOMEM
return vmaIterator{}, hostarch.AddrRange{}, droppedIDs, linuxerr.ENOMEM
}
if opts.MLockMode != memmap.MLockNone {
@@ -67,14 +73,14 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
if creds := auth.CredentialsFromContext(ctx); !creds.HasCapabilityIn(linux.CAP_IPC_LOCK, creds.UserNamespace.Root()) {
mlockLimit := limits.FromContext(ctx).Get(limits.MemoryLocked).Cur
if mlockLimit == 0 {
return vmaIterator{}, hostarch.AddrRange{}, linuxerr.EPERM
return vmaIterator{}, hostarch.AddrRange{}, droppedIDs, linuxerr.EPERM
}
newLockedAS := mm.lockedAS + opts.Length
if opts.Unmap {
newLockedAS -= mm.mlockedBytesRangeLocked(ar)
}
if newLockedAS > mlockLimit {
return vmaIterator{}, hostarch.AddrRange{}, linuxerr.EAGAIN
return vmaIterator{}, hostarch.AddrRange{}, droppedIDs, linuxerr.EAGAIN
}
}
}
@@ -84,7 +90,7 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
// file->f_op->mmap().
var vgap vmaGapIterator
if opts.Unmap {
vgap = mm.unmapLocked(ctx, ar)
vgap, droppedIDs = mm.unmapLocked(ctx, ar, droppedIDs)
} else {
vgap = mm.vmas.FindGap(ar.Start)
}
@@ -94,7 +100,7 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
// The expression for writable is vma.canWriteMappableLocked(), but we
// don't yet have a vma.
if err := opts.Mappable.AddMapping(ctx, mm, ar, opts.Offset, !opts.Private && opts.MaxPerms.Write); err != nil {
return vmaIterator{}, hostarch.AddrRange{}, err
return vmaIterator{}, hostarch.AddrRange{}, droppedIDs, err
}
}
@@ -128,7 +134,7 @@ func (mm *MemoryManager) createVMALocked(ctx context.Context, opts memmap.MMapOp
mm.lockedAS += opts.Length
}
return vseg, ar, nil
return vseg, ar, droppedIDs, nil
}
type findAvailableOpts struct {
@@ -345,11 +351,17 @@ const guardBytes = 256 * hostarch.PageSize
// unmapLocked unmaps all addresses in ar and returns the resulting gap in
// mm.vmas.
//
// Caller provides the droppedIDs slice to collect dropped mapping
// identities. The caller must drop the references on these identities outside a
// mm.mappingMu critical section. droppedIDs has append-like semantics, multiple
// calls to functions that drop mapping identities within a scope should reuse
// the same slice.
//
// Preconditions:
// * mm.mappingMu must be locked for writing.
// * ar.Length() != 0.
// * ar must be page-aligned.
func (mm *MemoryManager) unmapLocked(ctx context.Context, ar hostarch.AddrRange) vmaGapIterator {
func (mm *MemoryManager) unmapLocked(ctx context.Context, ar hostarch.AddrRange, droppedIDs []memmap.MappingIdentity) (vmaGapIterator, []memmap.MappingIdentity) {
if checkInvariants {
if !ar.WellFormed() || ar.Length() == 0 || !ar.IsPageAligned() {
panic(fmt.Sprintf("invalid ar: %v", ar))
@@ -359,24 +371,28 @@ func (mm *MemoryManager) unmapLocked(ctx context.Context, ar hostarch.AddrRange)
// AddressSpace mappings and pmas must be invalidated before
// mm.removeVMAsLocked() => memmap.Mappable.RemoveMapping().
mm.Invalidate(ar, memmap.InvalidateOpts{InvalidatePrivate: true})
return mm.removeVMAsLocked(ctx, ar)
return mm.removeVMAsLocked(ctx, ar, droppedIDs)
}
// removeVMAsLocked removes vmas for addresses in ar and returns the resulting
// gap in mm.vmas. It does not remove pmas or AddressSpace mappings; clients
// must do so before calling removeVMAsLocked.
// removeVMAsLocked removes vmas for addresses in ar and returns the
// resulting gap in mm.vmas.
//
// Caller provides the droppedIDs slice to collect dropped mapping
// identities. The caller must drop the references on these identities outside a
// mm.mappingMu critical section. droppedIDs has append-like semantics, multiple
// calls to functions that drop mapping identities within a scope should reuse
// the same slice.
//
// Preconditions:
// * mm.mappingMu must be locked for writing.
// * ar.Length() != 0.
// * ar must be page-aligned.
func (mm *MemoryManager) removeVMAsLocked(ctx context.Context, ar hostarch.AddrRange) vmaGapIterator {
func (mm *MemoryManager) removeVMAsLocked(ctx context.Context, ar hostarch.AddrRange, droppedIDs []memmap.MappingIdentity) (vmaGapIterator, []memmap.MappingIdentity) {
if checkInvariants {
if !ar.WellFormed() || ar.Length() == 0 || !ar.IsPageAligned() {
panic(fmt.Sprintf("invalid ar: %v", ar))
}
}
vseg, vgap := mm.vmas.Find(ar.Start)
if vgap.Ok() {
vseg = vgap.NextSegment()
@@ -389,7 +405,7 @@ func (mm *MemoryManager) removeVMAsLocked(ctx context.Context, ar hostarch.AddrR
vma.mappable.RemoveMapping(ctx, mm, vmaAR, vma.off, vma.canWriteMappableLocked())
}
if vma.id != nil {
vma.id.DecRef(ctx)
droppedIDs = append(droppedIDs, vma.id)
}
mm.usageAS -= uint64(vmaAR.Length())
if vma.isPrivateDataLocked() {
@@ -401,7 +417,7 @@ func (mm *MemoryManager) removeVMAsLocked(ctx context.Context, ar hostarch.AddrR
vgap = mm.vmas.Remove(vseg)
vseg = vgap.NextSegment()
}
return vgap
return vgap, droppedIDs
}
// canWriteMappableLocked returns true if it is possible for vma.mappable to be
@@ -459,6 +475,9 @@ func (vmaSetFunctions) Merge(ar1 hostarch.AddrRange, vma1 vma, ar2 hostarch.Addr
}
if vma2.id != nil {
// This DecRef() will never be the final ref, since the vma1 is
// currently holding a ref to the same mapping identity. Thus, we don't
// need to worry about whether we're in a mm.mappingMu critical section.
vma2.id.DecRef(context.Background())
}
return vma1, true