Call memmap.Mappable.Translate with more conservative usermem.AccessType.

MM.insertPMAsLocked() passes vma.maxPerms to memmap.Mappable.Translate
(although it unsets AccessType.Write if the vma is private). This
somewhat simplifies handling of pmas, since it means only COW-break
needs to replace existing pmas. However, it also means that a MAP_SHARED
mapping of a file opened O_RDWR dirties the file, regardless of the
mapping's permissions and whether or not the mapping is ever actually
written to with I/O that ignores permissions (e.g.
ptrace(PTRACE_POKEDATA)).

To fix this:

- Change the pma-getting path to request only the permissions that are
required for the calling access.

- Change memmap.Mappable.Translate to take requested permissions, and
return allowed permissions. This preserves the existing behavior in the
common cases where the memmap.Mappable isn't
fsutil.CachingInodeOperations and doesn't care if the translated
platform.File pages are written to.

- Change the MM.getPMAsLocked path to support permission upgrading of
pmas outside of copy-on-write.

PiperOrigin-RevId: 240196979
Change-Id: Ie0147c62c1fbc409467a6fa16269a413f3d7d571

pkg/sentry/fs/binder/binder.go

@@ -244,6 +244,7 @@ func (bp *Proc) Translate(ctx context.Context, required, optional memmap.Mappabl
 				Source: memmap.MappableRange{0, usermem.PageSize},
 				File:   bp.mfp.MemoryFile(),
 				Offset: bp.mapped.Start,
+				Perms:  usermem.AnyAccess,
 			},
 		}, err
 	}

pkg/sentry/fs/fsutil/host_mappable.go

@@ -94,6 +94,7 @@ func (h *HostMappable) Translate(ctx context.Context, required, optional memmap.
 			Source: optional,
 			File:   h,
 			Offset: optional.Start,
+			Perms:  usermem.AnyAccess,
 		},
 	}, nil
 }

pkg/sentry/fs/fsutil/inode_cached.go

@@ -739,6 +739,7 @@ func (c *CachingInodeOperations) Translate(ctx context.Context, required, option
 				Source: optional,
 				File:   c,
 				Offset: optional.Start,
+				Perms:  usermem.AnyAccess,
 			},
 		}, nil
 	}
@@ -768,16 +769,24 @@ func (c *CachingInodeOperations) Translate(ctx context.Context, required, option
 	var translatedEnd uint64
 	for seg := c.cache.FindSegment(required.Start); seg.Ok() && seg.Start() < required.End; seg, _ = seg.NextNonEmpty() {
 		segMR := seg.Range().Intersect(optional)
-		ts = append(ts, memmap.Translation{
-			Source: segMR,
-			File:   mf,
-			Offset: seg.FileRangeOf(segMR).Start,
-		})
+		// TODO: Make Translations writable even if writability is
+		// not required if already kept-dirty by another writable translation.
+		perms := usermem.AccessType{
+			Read:    true,
+			Execute: true,
+		}
 		if at.Write {
 			// From this point forward, this memory can be dirtied through the
 			// mapping at any time.
 			c.dirty.KeepDirty(segMR)
+			perms.Write = true
 		}
+		ts = append(ts, memmap.Translation{
+			Source: segMR,
+			File:   mf,
+			Offset: seg.FileRangeOf(segMR).Start,
+			Perms:  perms,
+		})
 		translatedEnd = segMR.End
 	}
 

pkg/sentry/fs/tmpfs/inode_file.go

@@ -481,6 +481,7 @@ func (f *fileInodeOperations) Translate(ctx context.Context, required, optional
 			Source: segMR,
 			File:   mf,
 			Offset: seg.FileRangeOf(segMR).Start,
+			Perms:  usermem.AnyAccess,
 		})
 		translatedEnd = segMR.End
 	}

pkg/sentry/kernel/shm/shm.go

@@ -455,6 +455,7 @@ func (s *Shm) Translate(ctx context.Context, required, optional memmap.MappableR
 				Source: source,
 				File:   s.mfp.MemoryFile(),
 				Offset: s.fr.Start + source.Start,
+				Perms:  usermem.AnyAccess,
 			},
 		}, err
 	}

pkg/sentry/memmap/memmap.go

@@ -105,13 +105,22 @@ type Translation struct {
 
 	// Offset is the offset into File at which this Translation begins.
 	Offset uint64
+
+	// Perms is the set of permissions for which platform.AddressSpace.MapFile
+	// and platform.AddressSpace.MapInternal on this Translation is permitted.
+	Perms usermem.AccessType
+}
+
+// FileRange returns the platform.FileRange represented by t.
+func (t Translation) FileRange() platform.FileRange {
+	return platform.FileRange{t.Offset, t.Offset + t.Source.Length()}
 }
 
 // CheckTranslateResult returns an error if (ts, terr) does not satisfy all
-// postconditions for Mappable.Translate(required, optional).
+// postconditions for Mappable.Translate(required, optional, at).
 //
 // Preconditions: As for Mappable.Translate.
-func CheckTranslateResult(required, optional MappableRange, ts []Translation, terr error) error {
+func CheckTranslateResult(required, optional MappableRange, at usermem.AccessType, ts []Translation, terr error) error {
 	// Verify that the inputs to Mappable.Translate were valid.
 	if !required.WellFormed() || required.Length() <= 0 {
 		panic(fmt.Sprintf("invalid required range: %v", required))
@@ -156,6 +165,10 @@ func CheckTranslateResult(required, optional MappableRange, ts []Translation, te
 		if !optional.IsSupersetOf(t.Source) {
 			return fmt.Errorf("Translation %+v lies outside optional range %v", t, optional)
 		}
+		// Each Translation must permit a superset of requested accesses.
+		if !t.Perms.SupersetOf(at) {
+			return fmt.Errorf("Translation %+v does not permit all requested access types %v", t, at)
+		}
 	}
 	// If the set of Translations does not cover the entire required range,
 	// Translate must return a non-nil error explaining why.

pkg/sentry/mm/address_space.go

@@ -179,7 +179,7 @@ func (mm *MemoryManager) mapASLocked(pseg pmaIterator, ar usermem.AddrRange, pre
 		pma := pseg.ValuePtr()
 		pmaAR := pseg.Range()
 		pmaMapAR := pmaAR.Intersect(mapAR)
-		perms := pma.vmaEffectivePerms
+		perms := pma.effectivePerms
 		if pma.needCOW {
 			perms.Write = false
 		}

pkg/sentry/mm/aio_context.go

@@ -302,6 +302,7 @@ func (m *aioMappable) Translate(ctx context.Context, required, optional memmap.M
 				Source: source,
 				File:   m.mfp.MemoryFile(),
 				Offset: m.fr.Start + source.Start,
+				Perms:  usermem.AnyAccess,
 			},
 		}, err
 	}

pkg/sentry/mm/debug.go

@@ -68,12 +68,12 @@ func (pseg pmaIterator) debugStringEntryLocked() []byte {
 	fmt.Fprintf(&b, "%08x-%08x ", pseg.Start(), pseg.End())
 
 	pma := pseg.ValuePtr()
-	if pma.vmaEffectivePerms.Read {
+	if pma.effectivePerms.Read {
 		b.WriteByte('r')
 	} else {
 		b.WriteByte('-')
 	}
-	if pma.vmaEffectivePerms.Write {
+	if pma.effectivePerms.Write {
 		if pma.needCOW {
 			b.WriteByte('c')
 		} else {
@@ -82,7 +82,7 @@ func (pseg pmaIterator) debugStringEntryLocked() []byte {
 	} else {
 		b.WriteByte('-')
 	}
-	if pma.vmaEffectivePerms.Execute {
+	if pma.effectivePerms.Execute {
 		b.WriteByte('x')
 	} else {
 		b.WriteByte('-')

pkg/sentry/mm/io.go

@@ -466,9 +466,7 @@ func (mm *MemoryManager) handleASIOFault(ctx context.Context, addr usermem.Addr,
 
 	// Ensure that we have usable pmas.
 	mm.activeMu.Lock()
-	pseg, pend, err := mm.getPMAsLocked(ctx, vseg, ar, pmaOpts{
-		breakCOW: at.Write,
-	})
+	pseg, pend, err := mm.getPMAsLocked(ctx, vseg, ar, at)
 	mm.mappingMu.RUnlock()
 	if pendaddr := pend.Start(); pendaddr < ar.End {
 		if pendaddr <= ar.Start {
@@ -498,14 +496,10 @@ func (mm *MemoryManager) handleASIOFault(ctx context.Context, addr usermem.Addr,
 //
 // Preconditions: 0 < ar.Length() <= math.MaxInt64.
 func (mm *MemoryManager) withInternalMappings(ctx context.Context, ar usermem.AddrRange, at usermem.AccessType, ignorePermissions bool, f func(safemem.BlockSeq) (uint64, error)) (int64, error) {
-	po := pmaOpts{
-		breakCOW: at.Write,
-	}
-
 	// If pmas are already available, we can do IO without touching mm.vmas or
 	// mm.mappingMu.
 	mm.activeMu.RLock()
-	if pseg := mm.existingPMAsLocked(ar, at, ignorePermissions, po, true /* needInternalMappings */); pseg.Ok() {
+	if pseg := mm.existingPMAsLocked(ar, at, ignorePermissions, true /* needInternalMappings */); pseg.Ok() {
 		n, err := f(mm.internalMappingsLocked(pseg, ar))
 		mm.activeMu.RUnlock()
 		// Do not convert errors returned by f to EFAULT.
@@ -526,7 +520,7 @@ func (mm *MemoryManager) withInternalMappings(ctx context.Context, ar usermem.Ad
 
 	// Ensure that we have usable pmas.
 	mm.activeMu.Lock()
-	pseg, pend, perr := mm.getPMAsLocked(ctx, vseg, ar, po)
+	pseg, pend, perr := mm.getPMAsLocked(ctx, vseg, ar, at)
 	mm.mappingMu.RUnlock()
 	if pendaddr := pend.Start(); pendaddr < ar.End {
 		if pendaddr <= ar.Start {
@@ -578,14 +572,10 @@ func (mm *MemoryManager) withVecInternalMappings(ctx context.Context, ars userme
 		return mm.withInternalMappings(ctx, ars.Head(), at, ignorePermissions, f)
 	}
 
-	po := pmaOpts{
-		breakCOW: at.Write,
-	}
-
 	// If pmas are already available, we can do IO without touching mm.vmas or
 	// mm.mappingMu.
 	mm.activeMu.RLock()
-	if mm.existingVecPMAsLocked(ars, at, ignorePermissions, po, true /* needInternalMappings */) {
+	if mm.existingVecPMAsLocked(ars, at, ignorePermissions, true /* needInternalMappings */) {
 		n, err := f(mm.vecInternalMappingsLocked(ars))
 		mm.activeMu.RUnlock()
 		// Do not convert errors returned by f to EFAULT.
@@ -603,7 +593,7 @@ func (mm *MemoryManager) withVecInternalMappings(ctx context.Context, ars userme
 
 	// Ensure that we have usable pmas.
 	mm.activeMu.Lock()
-	pars, perr := mm.getVecPMAsLocked(ctx, vars, po)
+	pars, perr := mm.getVecPMAsLocked(ctx, vars, at)
 	mm.mappingMu.RUnlock()
 	if pars.NumBytes() == 0 {
 		mm.activeMu.Unlock()

pkg/sentry/mm/lifecycle.go

@@ -124,7 +124,7 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
 		}
 		if !pma.needCOW {
 			pma.needCOW = true
-			if pma.vmaEffectivePerms.Write {
+			if pma.effectivePerms.Write {
 				// We don't want to unmap the whole address space, even though
 				// doing so would reduce calls to unmapASLocked(), because mm
 				// will most likely continue to be used after the fork, so
@@ -139,7 +139,9 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
 					}
 					unmapAR = srcpseg.Range()
 				}
+				pma.effectivePerms.Write = false
 			}
+			pma.maxPerms.Write = false
 		}
 		fr := srcpseg.fileRange()
 		mm2.incPrivateRef(fr)

pkg/sentry/mm/mm.go

@@ -71,9 +71,6 @@ type MemoryManager struct {
 	// ownership is shared by one or more pmas instead of being owned by a
 	// memmap.Mappable).
 	//
-	// NOTE: This should be replaced using refcounts on
-	// platform.File.
-	//
 	// privateRefs is immutable.
 	privateRefs *privateRefs
 
@@ -374,13 +371,27 @@ type pma struct {
 	file platform.File `state:"nosave"`
 
 	// off is the offset into file at which this pma begins.
+	//
+	// Note that pmas do *not* hold references on offsets in file! If private
+	// is true, MemoryManager.privateRefs holds the reference instead. If
+	// private is false, the corresponding memmap.Mappable holds the reference
+	// instead (per memmap.Mappable.Translate requirement).
 	off uint64
 
-	// vmaEffectivePerms and vmaMaxPerms are duplicated from the
-	// corresponding vma so that the IO implementation can avoid iterating
-	// mm.vmas when pmas already exist.
-	vmaEffectivePerms usermem.AccessType
-	vmaMaxPerms       usermem.AccessType
+	// translatePerms is the permissions returned by memmap.Mappable.Translate.
+	// If private is true, translatePerms is usermem.AnyAccess.
+	translatePerms usermem.AccessType
+
+	// effectivePerms is the permissions allowed for non-ignorePermissions
+	// accesses. maxPerms is the permissions allowed for ignorePermissions
+	// accesses. These are vma.effectivePerms and vma.maxPerms respectively,
+	// masked by pma.translatePerms and with Write disallowed if pma.needCOW is
+	// true.
+	//
+	// These are stored in the pma so that the IO implementation can avoid
+	// iterating mm.vmas when pmas already exist.
+	effectivePerms usermem.AccessType
+	maxPerms       usermem.AccessType
 
 	// needCOW is true if writes to the mapping must be propagated to a copy.
 	needCOW bool

pkg/sentry/mm/special_mappable.go

@@ -102,6 +102,7 @@ func (m *SpecialMappable) Translate(ctx context.Context, required, optional memm
 				Source: source,
 				File:   m.mfp.MemoryFile(),
 				Offset: m.fr.Start + source.Start,
+				Perms:  usermem.AnyAccess,
 			},
 		}, err
 	}

pkg/sentry/mm/syscalls.go

@@ -54,9 +54,7 @@ func (mm *MemoryManager) HandleUserFault(ctx context.Context, addr usermem.Addr,
 
 	// Ensure that we have a usable pma.
 	mm.activeMu.Lock()
-	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, pmaOpts{
-		breakCOW: at.Write,
-	})
+	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, at)
 	mm.mappingMu.RUnlock()
 	if err != nil {
 		mm.activeMu.Unlock()
@@ -186,7 +184,7 @@ func (mm *MemoryManager) populateVMA(ctx context.Context, vseg vmaIterator, ar u
 	}
 
 	// Ensure that we have usable pmas.
-	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, pmaOpts{})
+	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, usermem.NoAccess)
 	if err != nil {
 		// mm/util.c:vm_mmap_pgoff() ignores the error, if any, from
 		// mm/gup.c:mm_populate(). If it matters, we'll get it again when
@@ -231,7 +229,7 @@ func (mm *MemoryManager) populateVMAAndUnlock(ctx context.Context, vseg vmaItera
 	// mm.mappingMu doesn't need to be write-locked for getPMAsLocked, and it
 	// isn't needed at all for mapASLocked.
 	mm.mappingMu.DowngradeLock()
-	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, pmaOpts{})
+	pseg, _, err := mm.getPMAsLocked(ctx, vseg, ar, usermem.NoAccess)
 	mm.mappingMu.RUnlock()
 	if err != nil {
 		mm.activeMu.Unlock()
@@ -651,13 +649,17 @@ func (mm *MemoryManager) MProtect(addr usermem.Addr, length uint64, realPerms us
 		for pseg.Ok() && pseg.Start() < vseg.End() {
 			if pseg.Range().Overlaps(vseg.Range()) {
 				pseg = mm.pmas.Isolate(pseg, vseg.Range())
-				if !effectivePerms.SupersetOf(pseg.ValuePtr().vmaEffectivePerms) && !didUnmapAS {
+				pma := pseg.ValuePtr()
+				if !effectivePerms.SupersetOf(pma.effectivePerms) && !didUnmapAS {
 					// Unmap all of ar, not just vseg.Range(), to minimize host
 					// syscalls.
 					mm.unmapASLocked(ar)
 					didUnmapAS = true
 				}
-				pseg.ValuePtr().vmaEffectivePerms = effectivePerms
+				pma.effectivePerms = effectivePerms.Intersect(pma.translatePerms)
+				if pma.needCOW {
+					pma.effectivePerms.Write = false
+				}
 			}
 			pseg = pseg.NextSegment()
 		}
@@ -828,7 +830,7 @@ func (mm *MemoryManager) MLock(ctx context.Context, addr usermem.Addr, length ui
 				mm.mappingMu.RUnlock()
 				return syserror.ENOMEM
 			}
-			_, _, err := mm.getPMAsLocked(ctx, vseg, vseg.Range().Intersect(ar), pmaOpts{})
+			_, _, err := mm.getPMAsLocked(ctx, vseg, vseg.Range().Intersect(ar), usermem.NoAccess)
 			if err != nil {
 				mm.activeMu.Unlock()
 				mm.mappingMu.RUnlock()
@@ -923,7 +925,7 @@ func (mm *MemoryManager) MLockAll(ctx context.Context, opts MLockAllOpts) error
 		mm.mappingMu.DowngradeLock()
 		for vseg := mm.vmas.FirstSegment(); vseg.Ok(); vseg = vseg.NextSegment() {
 			if vseg.ValuePtr().effectivePerms.Any() {
-				mm.getPMAsLocked(ctx, vseg, vseg.Range(), pmaOpts{})
+				mm.getPMAsLocked(ctx, vseg, vseg.Range(), usermem.NoAccess)
 			}
 		}
 
@@ -981,7 +983,7 @@ func (mm *MemoryManager) Decommit(addr usermem.Addr, length uint64) error {
 		}
 		for pseg.Ok() && pseg.Start() < vsegAR.End {
 			pma := pseg.ValuePtr()
-			if pma.private && !mm.isPMACopyOnWriteLocked(pseg) {
+			if pma.private && !mm.isPMACopyOnWriteLocked(vseg, pseg) {
 				psegAR := pseg.Range().Intersect(ar)
 				if vsegAR.IsSupersetOf(psegAR) && vma.mappable == nil {
 					if err := mf.Decommit(pseg.fileRangeOf(psegAR)); err == nil {

pkg/sentry/usermem/access_type.go

@@ -93,6 +93,15 @@ func (a AccessType) Intersect(other AccessType) AccessType {
 	}
 }
 
+// Union returns the access types set in either a or other.
+func (a AccessType) Union(other AccessType) AccessType {
+	return AccessType{
+		Read:    a.Read || other.Read,
+		Write:   a.Write || other.Write,
+		Execute: a.Execute || other.Execute,
+	}
+}
+
 // Effective returns the set of effective access types allowed by a, even if
 // some types are not explicitly allowed.
 func (a AccessType) Effective() AccessType {