From f5462c934b150e7d1cabf08709f16658f1ddd450 Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Wed, 14 Feb 2024 09:34:36 -0800 Subject: [PATCH] Constrain MM.mapASLocked() to required range under PlatformEffectPopulate. PlatformEffectPopulate is currently only used by nvproxy; compared to switching to PlatformEffectCommit, this preserves the distinction of not using MAP_POPULATE if not requested by the application. PiperOrigin-RevId: 607013252 --- pkg/sentry/mm/address_space.go | 16 +++++++++------- pkg/sentry/mm/io.go | 3 ++- pkg/sentry/mm/syscalls.go | 26 +++++++++++++------------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pkg/sentry/mm/address_space.go b/pkg/sentry/mm/address_space.go index 3bd1d3589..c12898bf2 100644 --- a/pkg/sentry/mm/address_space.go +++ b/pkg/sentry/mm/address_space.go @@ -19,6 +19,7 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/hostarch" + "gvisor.dev/gvisor/pkg/sentry/memmap" "gvisor.dev/gvisor/pkg/sentry/platform" ) @@ -162,8 +163,7 @@ func (mm *MemoryManager) Deactivate() { mm.activeMu.Unlock() } -// mapASLocked maps addresses in ar into mm.as. If precommit is true, mappings -// for all addresses in ar should be precommitted. +// mapASLocked maps addresses in ar into mm.as. // // Preconditions: // - mm.activeMu must be locked. @@ -171,13 +171,15 @@ func (mm *MemoryManager) Deactivate() { // - ar.Length() != 0. // - ar must be page-aligned. // - pseg == mm.pmas.LowerBoundSegment(ar.Start). -func (mm *MemoryManager) mapASLocked(pseg pmaIterator, ar hostarch.AddrRange, precommit bool) error { +func (mm *MemoryManager) mapASLocked(pseg pmaIterator, ar hostarch.AddrRange, platformEffect memmap.MMapPlatformEffect) error { // By default, map entire pmas at a time, under the assumption that there // is no cost to mapping more of a pma than necessary. mapAR := hostarch.AddrRange{0, ^hostarch.Addr(hostarch.PageSize - 1)} - if precommit { - // When explicitly precommitting, only map ar, since overmapping may - // incur unexpected resource usage. + if platformEffect != memmap.PlatformEffectDefault { + // When explicitly committing, only map ar, since overmapping may incur + // unexpected resource usage. When explicitly populating, do the same + // since an underlying device file may be sensitive to the mapped + // range. mapAR = ar } else if mapUnit := mm.p.MapUnit(); mapUnit != 0 { // Limit the range we map to ar, aligned to mapUnit. @@ -205,7 +207,7 @@ func (mm *MemoryManager) mapASLocked(pseg pmaIterator, ar hostarch.AddrRange, pr perms.Write = false } if perms.Any() { // MapFile precondition - if err := mm.as.MapFile(pmaMapAR.Start, pma.file, pseg.fileRangeOf(pmaMapAR), perms, precommit); err != nil { + if err := mm.as.MapFile(pmaMapAR.Start, pma.file, pseg.fileRangeOf(pmaMapAR), perms, platformEffect == memmap.PlatformEffectCommit); err != nil { return err } } diff --git a/pkg/sentry/mm/io.go b/pkg/sentry/mm/io.go index ceb85791c..ea3a03a3e 100644 --- a/pkg/sentry/mm/io.go +++ b/pkg/sentry/mm/io.go @@ -19,6 +19,7 @@ import ( "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/safemem" + "gvisor.dev/gvisor/pkg/sentry/memmap" "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/pkg/usermem" ) @@ -499,7 +500,7 @@ func (mm *MemoryManager) handleASIOFault(ctx context.Context, addr hostarch.Addr // anymore. mm.activeMu.DowngradeLock() - err = mm.mapASLocked(pseg, ar, false) + err = mm.mapASLocked(pseg, ar, memmap.PlatformEffectDefault) mm.activeMu.RUnlock() return translateIOError(ctx, err) } diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index f33e0a567..1e84a7631 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -65,7 +65,7 @@ func (mm *MemoryManager) HandleUserFault(ctx context.Context, addr hostarch.Addr mm.activeMu.DowngradeLock() // Map the faulted page into the active AddressSpace. - err = mm.mapASLocked(pseg, ar, false) + err = mm.mapASLocked(pseg, ar, memmap.PlatformEffectDefault) mm.activeMu.RUnlock() return err } @@ -133,7 +133,7 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar switch { case opts.PlatformEffect >= memmap.PlatformEffectPopulate || opts.MLockMode == memmap.MLockEager: // Get pmas and map as requested. - mm.populateVMAAndUnlock(ctx, vseg, ar, opts.PlatformEffect == memmap.PlatformEffectCommit) + mm.populateVMAAndUnlock(ctx, vseg, ar, opts.PlatformEffect) case opts.Mappable == nil && length <= privateAllocUnit: // NOTE(b/63077076, b/63360184): Get pmas and map eagerly in the hope @@ -142,7 +142,7 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar // memmap.Mappable.Translate is unknown; and only for small mappings, // to avoid needing to allocate large amounts of memory that we may // subsequently need to checkpoint. - mm.populateVMAAndUnlock(ctx, vseg, ar, false) + mm.populateVMAAndUnlock(ctx, vseg, ar, memmap.PlatformEffectDefault) default: mm.mappingMu.Unlock() @@ -161,7 +161,7 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar // Preconditions: // - mm.mappingMu must be locked. // - vseg.Range().IsSupersetOf(ar). -func (mm *MemoryManager) populateVMA(ctx context.Context, vseg vmaIterator, ar hostarch.AddrRange, precommit bool) { +func (mm *MemoryManager) populateVMA(ctx context.Context, vseg vmaIterator, ar hostarch.AddrRange, platformEffect memmap.MMapPlatformEffect) { if !vseg.ValuePtr().effectivePerms.Any() { // Linux doesn't populate inaccessible pages. See // mm/gup.c:populate_vma_page_range. @@ -193,7 +193,7 @@ func (mm *MemoryManager) populateVMA(ctx context.Context, vseg vmaIterator, ar h mm.activeMu.DowngradeLock() // As above, errors are silently ignored. - mm.mapASLocked(pseg, ar, precommit) + mm.mapASLocked(pseg, ar, platformEffect) mm.activeMu.RUnlock() } @@ -208,7 +208,7 @@ func (mm *MemoryManager) populateVMA(ctx context.Context, vseg vmaIterator, ar h // // Postconditions: mm.mappingMu will be unlocked. // +checklocksrelease:mm.mappingMu -func (mm *MemoryManager) populateVMAAndUnlock(ctx context.Context, vseg vmaIterator, ar hostarch.AddrRange, precommit bool) { +func (mm *MemoryManager) populateVMAAndUnlock(ctx context.Context, vseg vmaIterator, ar hostarch.AddrRange, platformEffect memmap.MMapPlatformEffect) { // See populateVMA above for commentary. if !vseg.ValuePtr().effectivePerms.Any() { mm.mappingMu.Unlock() @@ -234,7 +234,7 @@ func (mm *MemoryManager) populateVMAAndUnlock(ctx context.Context, vseg vmaItera } mm.activeMu.DowngradeLock() - mm.mapASLocked(pseg, ar, precommit) + mm.mapASLocked(pseg, ar, platformEffect) mm.activeMu.RUnlock() } @@ -454,7 +454,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS }, droppedIDs) if err == nil { if vma.mlockMode == memmap.MLockEager { - mm.populateVMA(ctx, vseg, ar, true) + mm.populateVMA(ctx, vseg, ar, memmap.PlatformEffectCommit) } return oldAddr, nil } @@ -561,7 +561,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS if vma.mlockMode != memmap.MLockNone { mm.lockedAS += uint64(newAR.Length()) if vma.mlockMode == memmap.MLockEager { - mm.populateVMA(ctx, vseg, newAR, true) + mm.populateVMA(ctx, vseg, newAR, memmap.PlatformEffectCommit) } } return newAR.Start, nil @@ -605,7 +605,7 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS } if vma.mlockMode == memmap.MLockEager { - mm.populateVMA(ctx, vseg, newAR, true) + mm.populateVMA(ctx, vseg, newAR, memmap.PlatformEffectCommit) } return newAR.Start, nil @@ -796,7 +796,7 @@ func (mm *MemoryManager) Brk(ctx context.Context, addr hostarch.Addr) (hostarch. } mm.brk.End = addr if mm.defMLockMode == memmap.MLockEager { - mm.populateVMAAndUnlock(ctx, vseg, ar, true) + mm.populateVMAAndUnlock(ctx, vseg, ar, memmap.PlatformEffectCommit) } else { mm.mappingMu.Unlock() } @@ -909,7 +909,7 @@ func (mm *MemoryManager) MLock(ctx context.Context, addr hostarch.Addr, length u mm.mappingMu.RUnlock() if mm.as != nil { mm.activeMu.DowngradeLock() - err := mm.mapASLocked(mm.pmas.LowerBoundSegment(ar.Start), ar, true /* precommit */) + err := mm.mapASLocked(mm.pmas.LowerBoundSegment(ar.Start), ar, memmap.PlatformEffectCommit) mm.activeMu.RUnlock() if err != nil { return err @@ -993,7 +993,7 @@ func (mm *MemoryManager) MLockAll(ctx context.Context, opts MLockAllOpts) error mm.mappingMu.RUnlock() if mm.as != nil { mm.activeMu.DowngradeLock() - mm.mapASLocked(mm.pmas.FirstSegment(), mm.applicationAddrRange(), true /* precommit */) + mm.mapASLocked(mm.pmas.FirstSegment(), mm.applicationAddrRange(), memmap.PlatformEffectCommit) mm.activeMu.RUnlock() } else { mm.activeMu.Unlock()