summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFabricio Voznika <fvoznika@google.com>2019-01-31 12:53:00 -0800
committerShentubot <shentubot@google.com>2019-01-31 12:54:00 -0800
commita497f5ed5f97e4ad49ed60dd46f0146ae45eefd6 (patch)
tree8673e58e90036bc078eca13b548db123ae0a0c12
parentf1c1ee8a8ea6c0035aa799af3d7f5733fa2275d0 (diff)
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
-rw-r--r--pkg/sentry/fs/fsutil/BUILD1
-rw-r--r--pkg/sentry/fs/fsutil/host_mappable.go104
-rw-r--r--pkg/sentry/fs/fsutil/host_mappable_state.go22
-rw-r--r--pkg/sentry/fs/gofer/file.go3
-rw-r--r--pkg/sentry/fs/gofer/inode.go6
-rw-r--r--pkg/sentry/fs/gofer/session.go17
6 files changed, 91 insertions, 62 deletions
diff --git a/pkg/sentry/fs/fsutil/BUILD b/pkg/sentry/fs/fsutil/BUILD
index 7dff970ea..d41fc17cc 100644
--- a/pkg/sentry/fs/fsutil/BUILD
+++ b/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",
],
diff --git a/pkg/sentry/fs/fsutil/host_mappable.go b/pkg/sentry/fs/fsutil/host_mappable.go
index 4e4bcf4a4..340f8d288 100644
--- a/pkg/sentry/fs/fsutil/host_mappable.go
+++ b/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
+}
diff --git a/pkg/sentry/fs/fsutil/host_mappable_state.go b/pkg/sentry/fs/fsutil/host_mappable_state.go
deleted file mode 100644
index 765f1ec87..000000000
--- a/pkg/sentry/fs/fsutil/host_mappable_state.go
+++ /dev/null
@@ -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
-}
diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go
index 2181ddc68..2bb25daf1 100644
--- a/pkg/sentry/fs/gofer/file.go
+++ b/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))
}
diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go
index 043705c58..1dc0ca0db 100644
--- a/pkg/sentry/fs/gofer/inode.go
+++ b/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)})
}
diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go
index b5b1c8202..d626b86f5 100644
--- a/pkg/sentry/fs/gofer/session.go
+++ b/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)