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

3 files changed, 28 insertions, 284 deletions

diff --git a/pkg/sentry/platform/kvm/BUILD b/pkg/sentry/platform/kvm/BUILD
index 6e40b3177..b7bf88249 100644
--- a/pkg/sentry/platform/kvm/BUILD
+++ b/pkg/sentry/platform/kvm/BUILD
@@ -2,28 +2,6 @@ load("//tools/go_stateify:defs.bzl", "go_library", "go_test")
 
 package(licenses = ["notice"])
 
-load("//tools/go_generics:defs.bzl", "go_template_instance")
-
-go_template_instance(
-    name = "host_map_set",
-    out = "host_map_set.go",
-    consts = {
-        "minDegree": "15",
-    },
-    imports = {
-        "usermem": "gvisor.googlesource.com/gvisor/pkg/sentry/usermem",
-    },
-    package = "kvm",
-    prefix = "hostMap",
-    template = "//pkg/segment:generic_set",
-    types = {
-        "Key": "usermem.Addr",
-        "Range": "usermem.AddrRange",
-        "Value": "uintptr",
-        "Functions": "hostMapSetFunctions",
-    },
-)
-
 go_library(
     name = "kvm",
     srcs = [
@@ -36,8 +14,6 @@ go_library(
         "bluepill_fault.go",
         "bluepill_unsafe.go",
         "context.go",
-        "host_map.go",
-        "host_map_set.go",
         "kvm.go",
         "kvm_amd64.go",
         "kvm_amd64_unsafe.go",
diff --git a/pkg/sentry/platform/kvm/address_space.go b/pkg/sentry/platform/kvm/address_space.go
index 72e897a9a..6d8d8e65b 100644
--- a/pkg/sentry/platform/kvm/address_space.go
+++ b/pkg/sentry/platform/kvm/address_space.go
@@ -15,7 +15,6 @@
 package kvm
 
 import (
-	"reflect"
 	"sync"
 	"sync/atomic"
 
@@ -88,11 +87,6 @@ type addressSpace struct {
 
 	// dirtySet is the set of dirty vCPUs.
 	dirtySet *dirtySet
-
-	// files contains files mapped in the host address space.
-	//
-	// See host_map.go for more information.
-	files hostMap
 }
 
 // invalidate is the implementation for Invalidate.
@@ -118,6 +112,11 @@ func (as *addressSpace) Touch(c *vCPU) bool {
 	return as.dirtySet.mark(c)
 }
 
+type hostMapEntry struct {
+	addr   uintptr
+	length uintptr
+}
+
 func (as *addressSpace) mapHost(addr usermem.Addr, m hostMapEntry, at usermem.AccessType) (inv bool) {
 	for m.length > 0 {
 		physical, length, ok := translateToPhysical(m.addr)
@@ -158,100 +157,57 @@ func (as *addressSpace) mapHost(addr usermem.Addr, m hostMapEntry, at usermem.Ac
 	return inv
 }
 
-func (as *addressSpace) mapHostFile(addr usermem.Addr, fd int, fr platform.FileRange, at usermem.AccessType) error {
-	// Create custom host mappings.
-	ms, err := as.files.CreateMappings(usermem.AddrRange{
-		Start: addr,
-		End:   addr + usermem.Addr(fr.End-fr.Start),
-	}, at, fd, fr.Start)
-	if err != nil {
-		return err
-	}
-
-	inv := false
-	for _, m := range ms {
-		// The host mapped slices are guaranteed to be aligned.
-		prev := as.mapHost(addr, m, at)
-		inv = inv || prev
-		addr += usermem.Addr(m.length)
-	}
-	if inv {
-		as.invalidate()
-	}
-
-	return nil
-}
+// MapFile implements platform.AddressSpace.MapFile.
+func (as *addressSpace) MapFile(addr usermem.Addr, f platform.File, fr platform.FileRange, at usermem.AccessType, precommit bool) error {
+	as.mu.Lock()
+	defer as.mu.Unlock()
 
-func (as *addressSpace) mapFilemem(addr usermem.Addr, fr platform.FileRange, at usermem.AccessType, precommit bool) error {
-	// TODO: Lock order at the platform level is not sufficiently
-	// well-defined to guarantee that the caller (FileMem.MapInto) is not
-	// holding any locks that FileMem.MapInternal may take.
-
-	// Retrieve mappings for the underlying filemem. Note that the
-	// permissions here are largely irrelevant, since it corresponds to
-	// physical memory for the guest. We enforce the given access type
-	// below, in the guest page tables.
-	bs, err := as.filemem.MapInternal(fr, usermem.AccessType{
-		Read:  true,
-		Write: true,
+	// Get mappings in the sentry's address space, which are guaranteed to be
+	// valid as long as a reference is held on the mapped pages (which is in
+	// turn required by AddressSpace.MapFile precondition).
+	//
+	// If precommit is true, we will touch mappings to commit them, so ensure
+	// that mappings are readable from sentry context.
+	//
+	// We don't execute from application file-mapped memory, and guest page
+	// tables don't care if we have execute permission (but they do need pages
+	// to be readable).
+	bs, err := f.MapInternal(fr, usermem.AccessType{
+		Read:  at.Read || at.Execute || precommit,
+		Write: at.Write,
 	})
 	if err != nil {
 		return err
 	}
 
-	// Save the original range for invalidation.
-	orig := usermem.AddrRange{
-		Start: addr,
-		End:   addr + usermem.Addr(fr.End-fr.Start),
-	}
-
+	// Map the mappings in the sentry's address space (guest physical memory)
+	// into the application's address space (guest virtual memory).
 	inv := false
 	for !bs.IsEmpty() {
 		b := bs.Head()
 		bs = bs.Tail()
 		// Since fr was page-aligned, b should also be page-aligned. We do the
 		// lookup in our host page tables for this translation.
-		s := b.ToSlice()
 		if precommit {
+			s := b.ToSlice()
 			for i := 0; i < len(s); i += usermem.PageSize {
 				_ = s[i] // Touch to commit.
 			}
 		}
 		prev := as.mapHost(addr, hostMapEntry{
-			addr:   reflect.ValueOf(&s[0]).Pointer(),
-			length: uintptr(len(s)),
+			addr:   b.Addr(),
+			length: uintptr(b.Len()),
 		}, at)
 		inv = inv || prev
-		addr += usermem.Addr(len(s))
+		addr += usermem.Addr(b.Len())
 	}
 	if inv {
 		as.invalidate()
-		as.files.DeleteMapping(orig)
 	}
 
 	return nil
 }
 
-// MapFile implements platform.AddressSpace.MapFile.
-func (as *addressSpace) MapFile(addr usermem.Addr, fd int, fr platform.FileRange, at usermem.AccessType, precommit bool) error {
-	as.mu.Lock()
-	defer as.mu.Unlock()
-
-	// Create an appropriate mapping. If this is filemem, we don't create
-	// custom mappings for each in-application mapping. For files however,
-	// we create distinct mappings for each address space. Unfortunately,
-	// there's not a better way to manage this here. The file underlying
-	// this fd can change at any time, so we can't actually index the file
-	// and share between address space. Oh well. It's all referring to the
-	// same physical pages, hopefully we don't run out of address space.
-	if fd != int(as.filemem.File().Fd()) {
-		// N.B. precommit is ignored for host files.
-		return as.mapHostFile(addr, fd, fr, at)
-	}
-
-	return as.mapFilemem(addr, fr, at, precommit)
-}
-
 // Unmap unmaps the given range by calling pagetables.PageTables.Unmap.
 func (as *addressSpace) Unmap(addr usermem.Addr, length uint64) {
 	as.mu.Lock()
@@ -264,10 +220,6 @@ func (as *addressSpace) Unmap(addr usermem.Addr, length uint64) {
 	})
 	if prev {
 		as.invalidate()
-		as.files.DeleteMapping(usermem.AddrRange{
-			Start: addr,
-			End:   addr + usermem.Addr(length),
-		})
 
 		// Recycle any freed intermediate pages.
 		as.pageTables.Allocator.Recycle()
diff --git a/pkg/sentry/platform/kvm/host_map.go b/pkg/sentry/platform/kvm/host_map.go
deleted file mode 100644
index ee6a1a42d..000000000
--- a/pkg/sentry/platform/kvm/host_map.go
+++ /dev/null
@@ -1,184 +0,0 @@
-// Copyright 2018 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package kvm
-
-import (
-	"fmt"
-	"sync"
-	"syscall"
-
-	"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
-)
-
-type hostMap struct {
-	// mu protects below.
-	mu sync.RWMutex
-
-	// set contains host mappings.
-	set hostMapSet
-}
-
-type hostMapEntry struct {
-	addr   uintptr
-	length uintptr
-}
-
-// forEach iterates over all mappings in the given range.
-//
-// Precondition: segFn and gapFn must be non-nil.
-func (hm *hostMap) forEach(
-	r usermem.AddrRange,
-	segFn func(offset uint64, m hostMapEntry),
-	gapFn func(offset uint64, length uintptr) (uintptr, bool)) {
-
-	seg, gap := hm.set.Find(r.Start)
-	for {
-		if seg.Ok() && seg.Start() < r.End {
-			// A valid segment: pass information.
-			overlap := seg.Range().Intersect(r)
-			segOffset := uintptr(overlap.Start - seg.Start())
-			mapOffset := uint64(overlap.Start - r.Start)
-			segFn(mapOffset, hostMapEntry{
-				addr:   seg.Value() + segOffset,
-				length: uintptr(overlap.Length()),
-			})
-			seg, gap = seg.NextNonEmpty()
-		} else if gap.Ok() && gap.Start() < r.End {
-			// A gap: pass gap information.
-			overlap := gap.Range().Intersect(r)
-			mapOffset := uint64(overlap.Start - r.Start)
-			addr, ok := gapFn(mapOffset, uintptr(overlap.Length()))
-			if ok {
-				seg = hm.set.Insert(gap, overlap, addr)
-				seg, gap = seg.NextNonEmpty()
-			} else {
-				seg = gap.NextSegment()
-				gap = hostMapGapIterator{} // Invalid.
-			}
-		} else {
-			// Terminal.
-			break
-		}
-	}
-}
-
-func (hm *hostMap) createMappings(r usermem.AddrRange, at usermem.AccessType, fd int, offset uint64) (ms []hostMapEntry, err error) {
-	hm.forEach(r, func(mapOffset uint64, m hostMapEntry) {
-		// Replace any existing mappings.
-		_, _, errno := syscall.RawSyscall6(
-			syscall.SYS_MMAP,
-			m.addr,
-			m.length,
-			uintptr(at.Prot()),
-			syscall.MAP_FIXED|syscall.MAP_SHARED,
-			uintptr(fd),
-			uintptr(offset+mapOffset))
-		if errno != 0 && err == nil {
-			err = errno
-		}
-	}, func(mapOffset uint64, length uintptr) (uintptr, bool) {
-		// Create a new mapping.
-		addr, _, errno := syscall.RawSyscall6(
-			syscall.SYS_MMAP,
-			0,
-			length,
-			uintptr(at.Prot()),
-			syscall.MAP_SHARED,
-			uintptr(fd),
-			uintptr(offset+mapOffset))
-		if errno != 0 {
-			err = errno
-			return 0, false
-		}
-		return addr, true
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	// Collect all entries.
-	//
-	// We do this after the first iteration because some segments may have
-	// been merged in the above, and we'll return the simplest form. This
-	// also provides a basic sanity check in the form of no gaps.
-	hm.forEach(r, func(_ uint64, m hostMapEntry) {
-		ms = append(ms, m)
-	}, func(uint64, uintptr) (uintptr, bool) {
-		// Should not happen: we just mapped this above.
-		panic("unexpected gap")
-	})
-
-	return ms, nil
-}
-
-// CreateMappings creates a new set of host mapping entries.
-func (hm *hostMap) CreateMappings(r usermem.AddrRange, at usermem.AccessType, fd int, offset uint64) (ms []hostMapEntry, err error) {
-	hm.mu.Lock()
-	ms, err = hm.createMappings(r, at, fd, offset)
-	hm.mu.Unlock()
-	return
-}
-
-func (hm *hostMap) deleteMapping(r usermem.AddrRange) {
-	// Remove all the existing mappings.
-	hm.forEach(r, func(_ uint64, m hostMapEntry) {
-		_, _, errno := syscall.RawSyscall(
-			syscall.SYS_MUNMAP,
-			m.addr,
-			m.length,
-			0)
-		if errno != 0 {
-			// Should never happen.
-			panic(fmt.Sprintf("unmap error: %v", errno))
-		}
-	}, func(uint64, uintptr) (uintptr, bool) {
-		// Sometimes deleteMapping will be called on a larger range
-		// than physical mappings are defined. That's okay.
-		return 0, false
-	})
-
-	// Knock the entire range out.
-	hm.set.RemoveRange(r)
-}
-
-// DeleteMapping deletes the given range.
-func (hm *hostMap) DeleteMapping(r usermem.AddrRange) {
-	hm.mu.Lock()
-	hm.deleteMapping(r)
-	hm.mu.Unlock()
-}
-
-// hostMapSetFunctions is used in the implementation of mapSet.
-type hostMapSetFunctions struct{}
-
-func (hostMapSetFunctions) MinKey() usermem.Addr    { return 0 }
-func (hostMapSetFunctions) MaxKey() usermem.Addr    { return ^usermem.Addr(0) }
-func (hostMapSetFunctions) ClearValue(val *uintptr) { *val = 0 }
-
-func (hostMapSetFunctions) Merge(r1 usermem.AddrRange, addr1 uintptr, r2 usermem.AddrRange, addr2 uintptr) (uintptr, bool) {
-	if addr1+uintptr(r1.Length()) != addr2 {
-		return 0, false
-	}
-
-	// Since the two regions are contiguous in both the key space and the
-	// value space, we can just store a single segment with the first host
-	// virtual address; the logic above operates based on the size of the
-	// segments.
-	return addr1, true
-}
-
-func (hostMapSetFunctions) Split(r usermem.AddrRange, hostAddr uintptr, split usermem.Addr) (uintptr, uintptr) {
-	return hostAddr, hostAddr + uintptr(split-r.Start)
-}