Clarify the platform.File interface.

- Redefine some memmap.Mappable, platform.File, and platform.Memory
semantics in terms of File reference counts (no functional change).

- Make AddressSpace.MapFile take a platform.File instead of a raw FD,
and replace platform.File.MapInto with platform.File.FD. This allows
kvm.AddressSpace.MapFile to always use platform.File.MapInternal instead
of maintaining its own (redundant) cache of file mappings in the sentry
address space.

PiperOrigin-RevId: 238044504
Change-Id: Ib73a11e4275c0da0126d0194aa6c6017a9cef64f

4 files changed, 29 insertions, 42 deletions

diff --git a/pkg/sentry/fs/fsutil/README.md b/pkg/sentry/fs/fsutil/README.md
index d3780e9fa..6e677890c 100644
--- a/pkg/sentry/fs/fsutil/README.md
+++ b/pkg/sentry/fs/fsutil/README.md
@@ -108,9 +108,9 @@ The host then sends a `SIGSEGV` to the sentry because the address range [`A`,
 `A`+8) is not mapped on the host. The `SIGSEGV` indicates that the memory was
 accessed writable. The sentry looks up the vma associated with [`A`, `A`+8),
 finds the file that was mapped and its `CachingInodeOperations`. It then calls
-`CachingInodeOperations.MapInto` which allocates memory to back [`A`, `A`+8). It
-may choose to allocate more memory (i.e. do "readahead") to minimize subsequent
-faults.
+`CachingInodeOperations.Translate` which allocates memory to back [`A`, `A`+8).
+It may choose to allocate more memory (i.e. do "readahead") to minimize
+subsequent faults.
 
 Memory that is allocated comes from a host tmpfs file (see `filemem.FileMem`).
 The host tmpfs file memory is brought up to date with the contents of the mapped
@@ -138,12 +138,11 @@ memcpy(A, buffer, 4);
 ```
 
 Since the first process has already mapped and accessed the same region of the
-file writable, `CachingInodeOperations.MapInto` is called but re-maps the memory
-that has already been allocated (because the host mapping can be invalidated at
-any time) rather than allocating new memory. The address range [`A`, `A`+0x1000)
-reflects the same cached view of the file as the first process sees. For
-example, reading 8 bytes from the file from either process via read(2) starting
-at offset 0 returns a consistent "bbbbaaaa".
+file writable, `CachingInodeOperations.Translate` is called but returns the
+memory that has already been allocated rather than allocating new memory. The
+address range [`A`, `A`+0x1000) reflects the same cached view of the file as the
+first process sees. For example, reading 8 bytes from the file from either
+process via read(2) starting at offset 0 returns a consistent "bbbbaaaa".
 
 When this process no longer needs the shared memory, it may do:
 
diff --git a/pkg/sentry/fs/fsutil/host_mappable.go b/pkg/sentry/fs/fsutil/host_mappable.go
index 340f8d288..1bb5c6b6e 100644
--- a/pkg/sentry/fs/fsutil/host_mappable.go
+++ b/pkg/sentry/fs/fsutil/host_mappable.go
@@ -106,16 +106,16 @@ func (h *HostMappable) InvalidateUnsavable(ctx context.Context) error {
 	return nil
 }
 
-// MapInto implements platform.File.MapInto.
-func (h *HostMappable) MapInto(as platform.AddressSpace, addr usermem.Addr, fr platform.FileRange, at usermem.AccessType, precommit bool) error {
-	return as.MapFile(addr, h.backingFile.FD(), fr, at, precommit)
-}
-
 // MapInternal implements platform.File.MapInternal.
 func (h *HostMappable) MapInternal(fr platform.FileRange, at usermem.AccessType) (safemem.BlockSeq, error) {
 	return h.hostFileMapper.MapInternal(fr, h.backingFile.FD(), at.Write)
 }
 
+// FD implements platform.File.FD.
+func (h *HostMappable) FD() int {
+	return h.backingFile.FD()
+}
+
 // IncRef implements platform.File.IncRef.
 func (h *HostMappable) IncRef(fr platform.FileRange) {
 	mr := memmap.MappableRange{Start: fr.Start, End: fr.End}
diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go
index e3b52e943..ef11676b8 100644
--- a/pkg/sentry/fs/fsutil/inode_cached.go
+++ b/pkg/sentry/fs/fsutil/inode_cached.go
@@ -835,20 +835,6 @@ func (c *CachingInodeOperations) InvalidateUnsavable(ctx context.Context) error
 	return nil
 }
 
-// MapInto implements platform.File.MapInto. This is used when we directly map
-// an underlying host fd and CachingInodeOperations is used as the platform.File
-// during translation.
-func (c *CachingInodeOperations) MapInto(as platform.AddressSpace, addr usermem.Addr, fr platform.FileRange, at usermem.AccessType, precommit bool) error {
-	return as.MapFile(addr, c.backingFile.FD(), fr, at, precommit)
-}
-
-// MapInternal implements platform.File.MapInternal. This is used when we
-// directly map an underlying host fd and CachingInodeOperations is used as the
-// platform.File during translation.
-func (c *CachingInodeOperations) MapInternal(fr platform.FileRange, at usermem.AccessType) (safemem.BlockSeq, error) {
-	return c.hostFileMapper.MapInternal(fr, c.backingFile.FD(), at.Write)
-}
-
 // IncRef implements platform.File.IncRef. This is used when we directly map an
 // underlying host fd and CachingInodeOperations is used as the platform.File
 // during translation.
@@ -900,3 +886,17 @@ func (c *CachingInodeOperations) DecRef(fr platform.FileRange) {
 	c.dataMu.Unlock()
 
 }
+
+// MapInternal implements platform.File.MapInternal. This is used when we
+// directly map an underlying host fd and CachingInodeOperations is used as the
+// platform.File during translation.
+func (c *CachingInodeOperations) MapInternal(fr platform.FileRange, at usermem.AccessType) (safemem.BlockSeq, error) {
+	return c.hostFileMapper.MapInternal(fr, c.backingFile.FD(), at.Write)
+}
+
+// FD implements platform.File.FD. This is used when we directly map an
+// underlying host fd and CachingInodeOperations is used as the platform.File
+// during translation.
+func (c *CachingInodeOperations) FD() int {
+	return c.backingFile.FD()
+}
diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go
index 5648ff8f4..13d06684d 100644
--- a/pkg/sentry/fs/tmpfs/inode_file.go
+++ b/pkg/sentry/fs/tmpfs/inode_file.go
@@ -39,20 +39,8 @@ var (
 )
 
 // fileInodeOperations implements fs.InodeOperations for a regular tmpfs file.
-// These files are backed by FrameRegions allocated from a platform.Memory,
-// and may be directly mapped.
-//
-// The tmpfs file memory is backed by FrameRegions, each of which is reference
-// counted. frames maintains a single reference on each of the FrameRegions.
-// Since these contain the contents of the file, the reference may only be
-// decremented once this file is both deleted and all handles to the file have
-// been closed.
-//
-// Mappable users may also call IncRefOn/DecRefOn, generally to indicate that
-// they plan to use MapInto to map the file into an AddressSpace. These calls
-// include an InvalidatorRegion associated with that reference. When the
-// referenced portion of the file is removed (with Truncate), the associated
-// InvalidatorRegion is invalidated.
+// These files are backed by pages allocated from a platform.Memory, and may be
+// directly mapped.
 //
 // +stateify savable
 type fileInodeOperations struct {