diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index b2aa930cb..2e59e415d 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -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) } } diff --git a/pkg/sentry/mm/shm.go b/pkg/sentry/mm/shm.go index 94d5112a1..2a7e472ff 100644 --- a/pkg/sentry/mm/shm.go +++ b/pkg/sentry/mm/shm.go @@ -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() } diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index 7b1865567..e4c889081 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -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: diff --git a/pkg/sentry/mm/vma.go b/pkg/sentry/mm/vma.go index e0e80a8c7..ad1b964a3 100644 --- a/pkg/sentry/mm/vma.go +++ b/pkg/sentry/mm/vma.go @@ -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