From bdc50df4596ae948685e26012094df3fd214dbfa Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Mon, 30 Oct 2023 13:33:30 -0700 Subject: [PATCH] Split memmap.MMapOpts.Precommit into an enum. `Precommit=false` allows MM to delay creating PMAs (roughly speaking, host mmaps) until they are needed (e.g. page faults). `Precommit=true` requires MM to create PMAs, but also requires them to be precommitted (roughly speaking, mmapped with MAP_POPULATE). Introduce an intermediate setting, memmap.PlatformEffectPopulate, that requires MM to create PMAs but does not require precommit. Nothing in this CL uses the new setting, so this should result in no functional change. Updates #9593 PiperOrigin-RevId: 577939908 --- pkg/sentry/memmap/memmap.go | 23 ++++++++++++++++++++--- pkg/sentry/mm/syscalls.go | 6 +++--- pkg/sentry/syscalls/linux/sys_mmap.go | 4 +++- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pkg/sentry/memmap/memmap.go b/pkg/sentry/memmap/memmap.go index 5537fd639..adca2d8ef 100644 --- a/pkg/sentry/memmap/memmap.go +++ b/pkg/sentry/memmap/memmap.go @@ -356,9 +356,7 @@ type MMapOpts struct { // downward on guard page faults. GrowsDown bool - // Precommit is true if the platform should eagerly commit resources to the - // mapping (see platform.AddressSpace.MapFile). - Precommit bool + PlatformEffect MMapPlatformEffect // MLockMode specifies the memory locking behavior of the mapping. MLockMode MLockMode @@ -382,6 +380,25 @@ type MMapOpts struct { SentryOwnedContent bool } +// MMapPlatformEffect is the type of MMapOpts.PlatformEffect. +type MMapPlatformEffect uint8 + +// Possible values for MMapOpts.PlatformEffect: +const ( + // PlatformEffectDefault indicates that no specific behavior is requested + // from the platform. + PlatformEffectDefault MMapPlatformEffect = iota + + // PlatformEffectPopulate indicates that platform mappings should be + // established for all pages in the mapping. + PlatformEffectPopulate + + // PlatformEffectCommit is like PlatformEffectPopulate, but also requests + // that the platform eagerly commit resources to the mapping, as in + // platform.AddressSpace.MapFile(precommit=true). + PlatformEffectCommit +) + // File represents a host file that may be mapped into an platform.AddressSpace. type File interface { // All pages in a File are reference-counted. diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index 884964e8c..dceae9c17 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -131,9 +131,9 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (hostar // mm/util.c:vm_mmap_pgoff() => mm/gup.c:__mm_populate() => // populate_vma_page_range(). Confirm this behavior. switch { - case opts.Precommit || opts.MLockMode == memmap.MLockEager: - // Get pmas and map with precommit as requested. - mm.populateVMAAndUnlock(ctx, vseg, ar, true) + case opts.PlatformEffect >= memmap.PlatformEffectPopulate || opts.MLockMode == memmap.MLockEager: + // Get pmas and map as requested. + mm.populateVMAAndUnlock(ctx, vseg, ar, opts.PlatformEffect == memmap.PlatformEffectCommit) case opts.Mappable == nil && length <= privateAllocUnit: // NOTE(b/63077076, b/63360184): Get pmas and map eagerly in the hope diff --git a/pkg/sentry/syscalls/linux/sys_mmap.go b/pkg/sentry/syscalls/linux/sys_mmap.go index 50ecdcaf2..2da71232f 100644 --- a/pkg/sentry/syscalls/linux/sys_mmap.go +++ b/pkg/sentry/syscalls/linux/sys_mmap.go @@ -67,7 +67,9 @@ func Mmap(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, * }, MaxPerms: hostarch.AnyAccess, GrowsDown: linux.MAP_GROWSDOWN&flags != 0, - Precommit: linux.MAP_POPULATE&flags != 0, + } + if linux.MAP_POPULATE&flags != 0 { + opts.PlatformEffect = memmap.PlatformEffectCommit } if linux.MAP_LOCKED&flags != 0 { opts.MLockMode = memmap.MLockEager