Invalidate COW mappings when file is truncated

This changed required making fsutil.HostMappable use
a backing file to ensure the correct FD would be used
for read/write operations.

RELNOTES: relnotes is needed for the parent CL.
PiperOrigin-RevId: 231836164
Change-Id: I8ae9639715529874ea7d80a65e2c711a5b4ce254

pkg/sentry/fs/fsutil/BUILD

@@ -71,7 +71,6 @@ go_library(
         "host_file_mapper_state.go",
         "host_file_mapper_unsafe.go",
         "host_mappable.go",
-        "host_mappable_state.go",
         "inode.go",
         "inode_cached.go",
     ],

pkg/sentry/fs/fsutil/host_mappable.go

@@ -15,57 +15,50 @@
 package fsutil
 
 import (
+	"math"
 	"sync"
 
 	"gvisor.googlesource.com/gvisor/pkg/sentry/context"
+	"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/memmap"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/safemem"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
 )
 
-// HostMappable implements memmap.Mappable and platform.File over an arbitrary
-// host file descriptor.
+// HostMappable implements memmap.Mappable and platform.File over a
+// CachedFileObject.
+//
+// Lock order (compare the lock order model in mm/mm.go):
+//   truncateMu ("fs locks")
+//     mu ("memmap.Mappable locks not taken by Translate")
+//       ("platform.File locks")
+//   	     backingFile ("CachedFileObject locks")
 //
 // +stateify savable
 type HostMappable struct {
 	hostFileMapper *HostFileMapper
 
-	mu sync.Mutex `state:"nosave"`
+	backingFile CachedFileObject
 
-	// fd is the file descriptor to the host. Protected by mu.
-	fd int `state:"nosave"`
+	mu sync.Mutex `state:"nosave"`
 
 	// mappings tracks mappings of the cached file object into
 	// memmap.MappingSpaces so it can invalidated upon save. Protected by mu.
 	mappings memmap.MappingSet
+
+	// truncateMu protects writes and truncations. See Truncate() for details.
+	truncateMu sync.RWMutex `state:"nosave"`
 }
 
 // NewHostMappable creates a new mappable that maps directly to host FD.
-func NewHostMappable() *HostMappable {
+func NewHostMappable(backingFile CachedFileObject) *HostMappable {
 	return &HostMappable{
 		hostFileMapper: NewHostFileMapper(),
-		fd:             -1,
+		backingFile:    backingFile,
 	}
 }
 
-func (h *HostMappable) getFD() int {
-	h.mu.Lock()
-	defer h.mu.Unlock()
-	if h.fd < 0 {
-		panic("HostMappable FD isn't set")
-	}
-	return h.fd
-}
-
-// UpdateFD sets the host FD iff FD hasn't been set before or if there are
-// no mappings.
-func (h *HostMappable) UpdateFD(fd int) {
-	h.mu.Lock()
-	defer h.mu.Unlock()
-	h.fd = fd
-}
-
 // AddMapping implements memmap.Mappable.AddMapping.
 func (h *HostMappable) AddMapping(ctx context.Context, ms memmap.MappingSpace, ar usermem.AddrRange, offset uint64, writable bool) error {
 	// Hot path. Avoid defers.
@@ -115,12 +108,12 @@ func (h *HostMappable) InvalidateUnsavable(ctx context.Context) error {
 
 // 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.getFD(), fr, at, precommit)
+	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.getFD(), at.Write)
+	return h.hostFileMapper.MapInternal(fr, h.backingFile.FD(), at.Write)
 }
 
 // IncRef implements platform.File.IncRef.
@@ -134,3 +127,62 @@ func (h *HostMappable) DecRef(fr platform.FileRange) {
 	mr := memmap.MappableRange{Start: fr.Start, End: fr.End}
 	h.hostFileMapper.DecRefOn(mr)
 }
+
+// Truncate truncates the file, invalidating any mapping that may have been
+// removed after the size change.
+//
+// Truncation and writes are synchronized to prevent races where writes make the
+// file grow between truncation and invalidation below:
+//   T1: Calls SetMaskedAttributes and stalls
+//   T2: Appends to file causing it to grow
+//   T2: Writes to mapped pages and COW happens
+//   T1: Continues and wronly invalidates the page mapped in step above.
+func (h *HostMappable) Truncate(ctx context.Context, newSize int64) error {
+	h.truncateMu.Lock()
+	defer h.truncateMu.Unlock()
+
+	mask := fs.AttrMask{Size: true}
+	attr := fs.UnstableAttr{Size: newSize}
+	if err := h.backingFile.SetMaskedAttributes(ctx, mask, attr); err != nil {
+		return err
+	}
+
+	// Invalidate COW mappings that may exist beyond the new size in case the file
+	// is being shrunk. Other mappinsg don't need to be invalidated because
+	// translate will just return identical mappings after invalidation anyway,
+	// and SIGBUS will be raised and handled when the mappings are touched.
+	//
+	// Compare Linux's mm/truncate.c:truncate_setsize() =>
+	// truncate_pagecache() =>
+	// mm/memory.c:unmap_mapping_range(evencows=1).
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	mr := memmap.MappableRange{
+		Start: fs.OffsetPageEnd(newSize),
+		End:   fs.OffsetPageEnd(math.MaxInt64),
+	}
+	h.mappings.Invalidate(mr, memmap.InvalidateOpts{InvalidatePrivate: true})
+
+	return nil
+}
+
+// Write writes to the file backing this mappable.
+func (h *HostMappable) Write(ctx context.Context, src usermem.IOSequence, offset int64) (int64, error) {
+	h.truncateMu.RLock()
+	n, err := src.CopyInTo(ctx, &writer{ctx: ctx, hostMappable: h, off: offset})
+	h.truncateMu.RUnlock()
+	return n, err
+}
+
+type writer struct {
+	ctx          context.Context
+	hostMappable *HostMappable
+	off          int64
+}
+
+// WriteFromBlocks implements safemem.Writer.WriteFromBlocks.
+func (w *writer) WriteFromBlocks(src safemem.BlockSeq) (uint64, error) {
+	n, err := w.hostMappable.backingFile.WriteFromBlocksAt(w.ctx, src, uint64(w.off))
+	w.off += int64(n)
+	return n, err
+}

pkg/sentry/fs/fsutil/host_mappable_state.go

@@ -1,22 +0,0 @@
-// Copyright 2019 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 fsutil
-
-// afterLoad is invoked by stateify.
-func (h *HostMappable) afterLoad() {
-	h.mu.Lock()
-	defer h.mu.Unlock()
-	h.fd = -1
-}

pkg/sentry/fs/gofer/file.go

@@ -215,6 +215,9 @@ func (f *fileOperations) Write(ctx context.Context, file *fs.File, src usermem.I
 		}
 		return n, err
 	}
+	if f.inodeOperations.fileState.hostMappable != nil {
+		return f.inodeOperations.fileState.hostMappable.Write(ctx, src, offset)
+	}
 	return src.CopyInTo(ctx, f.handles.readWriterAt(ctx, offset))
 }
 

pkg/sentry/fs/gofer/inode.go

@@ -170,9 +170,6 @@ func (i *inodeFileState) setHandlesForCachedIO(flags fs.FileFlags, h *handles) {
 			i.writebackRW = true
 		}
 	}
-	if i.hostMappable != nil {
-		i.hostMappable.UpdateFD(i.fdLocked())
-	}
 }
 
 // getCachedHandles returns any cached handles which would accelerate
@@ -520,6 +517,9 @@ func (i *inodeOperations) Truncate(ctx context.Context, inode *fs.Inode, length
 	if i.session().cachePolicy.useCachingInodeOps(inode) {
 		return i.cachingInodeOps.Truncate(ctx, inode, length)
 	}
+	if i.session().cachePolicy == cacheRemoteRevalidating {
+		return i.fileState.hostMappable.Truncate(ctx, length)
+	}
 
 	return i.fileState.file.setAttr(ctx, p9.SetAttrMask{Size: true}, p9.SetAttr{Size: uint64(length)})
 }

pkg/sentry/fs/gofer/session.go

@@ -197,17 +197,14 @@ func newInodeOperations(ctx context.Context, s *session, file contextFile, qid p
 		}
 	}
 
-	var hm *fsutil.HostMappable
-	if s.cachePolicy == cacheRemoteRevalidating && fs.IsFile(sattr) {
-		hm = fsutil.NewHostMappable()
-	}
-
 	fileState := &inodeFileState{
-		s:            s,
-		file:         file,
-		sattr:        sattr,
-		key:          deviceKey,
-		hostMappable: hm,
+		s:     s,
+		file:  file,
+		sattr: sattr,
+		key:   deviceKey,
+	}
+	if s.cachePolicy == cacheRemoteRevalidating && fs.IsFile(sattr) {
+		fileState.hostMappable = fsutil.NewHostMappable(fileState)
 	}
 
 	uattr := unstable(ctx, valid, attr, s.mounter, s.client)