From f3723f805989d0c5956fbb6aa88b1cd9ac20753c Mon Sep 17 00:00:00 2001
From: Jamie Liu <jamieliu@google.com>
Date: Mon, 25 Mar 2019 12:41:36 -0700
Subject: 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        |  1 +
 pkg/sentry/fs/fsutil/host_mappable.go |  1 +
 pkg/sentry/fs/fsutil/inode_cached.go  | 19 ++++++++++++++-----
 pkg/sentry/fs/tmpfs/inode_file.go     |  1 +
 4 files changed, 17 insertions(+), 5 deletions(-)

(limited to 'pkg/sentry/fs')

diff --git a/pkg/sentry/fs/binder/binder.go b/pkg/sentry/fs/binder/binder.go
index 16fb4806f..188353961 100644
--- a/pkg/sentry/fs/binder/binder.go
+++ b/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
 	}
diff --git a/pkg/sentry/fs/fsutil/host_mappable.go b/pkg/sentry/fs/fsutil/host_mappable.go
index 1bb5c6b6e..4a182baa1 100644
--- a/pkg/sentry/fs/fsutil/host_mappable.go
+++ b/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
 }
diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go
index 9bd923678..6ca51ab0d 100644
--- a/pkg/sentry/fs/fsutil/inode_cached.go
+++ b/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
 	}
 
diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go
index 3c84b2977..25bf2b9dd 100644
--- a/pkg/sentry/fs/tmpfs/inode_file.go
+++ b/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
 	}
-- 
cgit v1.2.3