diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go index ef90c5915..9125cf6fd 100644 --- a/pkg/sentry/fsimpl/gofer/regular_file.go +++ b/pkg/sentry/fsimpl/gofer/regular_file.go @@ -439,7 +439,7 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) End: gapEnd, } optMR := gap.Range() - _, err := rw.d.cache.Fill(rw.ctx, reqMR, maxFillRange(reqMR, optMR), rw.d.size.Load(), mf, usage.PageCache, true /* populate */, h.readToBlocksAt) + _, err := rw.d.cache.Fill(rw.ctx, reqMR, maxFillRange(reqMR, optMR), rw.d.size.Load(), mf, usage.PageCache, pgalloc.AllocateAndWritePopulate, h.readToBlocksAt) mf.MarkEvictable(rw.d, pgalloc.EvictableRange{optMR.Start, optMR.End}) seg, gap = rw.d.cache.Find(rw.off) if !seg.Ok() { @@ -798,7 +798,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab mf := d.fs.mfp.MemoryFile() h := d.readHandle() - _, cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, true /* populate */, h.readToBlocksAt) + _, cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, pgalloc.AllocateAndWritePopulate, h.readToBlocksAt) var ts []memmap.Translation var translatedEnd uint64 diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 49f3dc3ff..c090b00e0 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -307,7 +307,7 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap. } optional = required } - pagesAlloced, cerr := rf.data.Fill(ctx, required, optional, rf.size.RacyLoad(), rf.inode.fs.mf, rf.memoryUsageKind, false /* populate */, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { + pagesAlloced, cerr := rf.data.Fill(ctx, required, optional, rf.size.RacyLoad(), rf.inode.fs.mf, rf.memoryUsageKind, pgalloc.AllocateOnly, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { // Newly-allocated pages are zeroed, so we don't need to do anything. return dsts.NumBytes(), nil }) @@ -385,10 +385,9 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint if !f.inode.fs.accountPages(pagesToFill) { return linuxerr.ENOSPC } - // Pass populate = true here despite the fact that we don't touch these pages - // both for consistency with the expected behavior of fallocate(2) and in - // expectation of a future write to them. - pagesAlloced, err := f.data.Fill(ctx, required, required, newSize, f.inode.fs.mf, f.memoryUsageKind, true /* populate */, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { + // Given our definitions in pgalloc, fallocate(2) semantics imply that pages + // in the MemoryFile must be committed, in addition to being allocated. + pagesAlloced, err := f.data.Fill(ctx, required, required, newSize, f.inode.fs.mf, f.memoryUsageKind, pgalloc.AllocateAndCommit, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { // Newly-allocated pages are zeroed, so we don't need to do anything. return dsts.NumBytes(), nil }) diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index ba1b7d4f6..d79f52605 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -162,12 +162,18 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt newFSType = tmpfsOpts.FilesystemType } if tmpfsOpts.FilestoreFD != nil { - // DecommitOnDestroy because tmpfsOpts.FilestoreFD may be backed by a - // host filesystem based file, which needs to be decommited on destroy. - // DisableIMAWorkAround because sentry's seccomp filters don't allow the - // mmap(2) syscalls that this work around uses. User of this feature is - // expected to have performed the work around outside the sandbox. - mfOpts := pgalloc.MemoryFileOpts{DecommitOnDestroy: true, DisableIMAWorkAround: true} + mfOpts := pgalloc.MemoryFileOpts{ + // tmpfsOpts.FilestoreFD may be backed by a file on disk (not memfd), + // which needs to be decommited on destroy to release disk space. + DecommitOnDestroy: true, + // sentry's seccomp filters don't allow the mmap(2) syscalls that + // pgalloc.IMAWorkAroundForMemFile() uses. Users of tmpfsOpts.FilestoreFD + // are expected to have performed the work around outside the sandbox. + DisableIMAWorkAround: true, + // Custom filestore FDs are usually backed by files on disk. Ideally we + // would confirm with fstatfs(2) but that is prohibited by seccomp. + DiskBackedFile: true, + } var err error mf, err = pgalloc.NewMemoryFile(tmpfsOpts.FilestoreFD.ReleaseToFile("overlay-filestore"), mfOpts) if err != nil { diff --git a/pkg/sentry/fsutil/file_range_set.go b/pkg/sentry/fsutil/file_range_set.go index 3f1bdcd83..46736fbd0 100644 --- a/pkg/sentry/fsutil/file_range_set.go +++ b/pkg/sentry/fsutil/file_range_set.go @@ -111,7 +111,7 @@ func (frs *FileRangeSet) PagesToFill(required, optional memmap.MappableRange) ui // - required.Length() > 0. // - optional.IsSupersetOf(required). // - required and optional must be page-aligned. -func (frs *FileRangeSet) Fill(ctx context.Context, required, optional memmap.MappableRange, fileSize uint64, mf *pgalloc.MemoryFile, kind usage.MemoryKind, populate bool, readAt func(ctx context.Context, dsts safemem.BlockSeq, offset uint64) (uint64, error)) (uint64, error) { +func (frs *FileRangeSet) Fill(ctx context.Context, required, optional memmap.MappableRange, fileSize uint64, mf *pgalloc.MemoryFile, kind usage.MemoryKind, allocMode pgalloc.AllocationMode, readAt func(ctx context.Context, dsts safemem.BlockSeq, offset uint64) (uint64, error)) (uint64, error) { gap := frs.LowerBoundGap(required.Start) var pagesAlloced uint64 for gap.Ok() && gap.Start() < required.End { @@ -122,7 +122,7 @@ func (frs *FileRangeSet) Fill(ctx context.Context, required, optional memmap.Map gr := gap.Range().Intersect(optional) // Read data into the gap. - fr, err := mf.AllocateAndFill(gr.Length(), kind, populate, safemem.ReaderFunc(func(dsts safemem.BlockSeq) (uint64, error) { + fr, err := mf.AllocateAndFill(gr.Length(), kind, allocMode, safemem.ReaderFunc(func(dsts safemem.BlockSeq) (uint64, error) { var done uint64 for !dsts.IsEmpty() { n, err := func() (uint64, error) { diff --git a/pkg/sentry/mm/pma.go b/pkg/sentry/mm/pma.go index 40f04f525..8ddf42db7 100644 --- a/pkg/sentry/mm/pma.go +++ b/pkg/sentry/mm/pma.go @@ -373,7 +373,7 @@ func (mm *MemoryManager) getPMAsInternalLocked(ctx context.Context, vseg vmaIter return pstart, pseg.PrevGap(), err } // Copy contents. - fr, err := mf.AllocateAndFill(uint64(copyAR.Length()), usage.Anonymous, true /* populate */, &safemem.BlockSeqReader{mm.internalMappingsLocked(pseg, copyAR)}) + fr, err := mf.AllocateAndFill(uint64(copyAR.Length()), usage.Anonymous, pgalloc.AllocateAndWritePopulate, &safemem.BlockSeqReader{mm.internalMappingsLocked(pseg, copyAR)}) if _, ok := err.(safecopy.BusError); ok { // If we got SIGBUS during the copy, deliver SIGBUS to // userspace (instead of SIGSEGV) if we're breaking diff --git a/pkg/sentry/pgalloc/pgalloc.go b/pkg/sentry/pgalloc/pgalloc.go index 30b714c1e..40fd67aff 100644 --- a/pkg/sentry/pgalloc/pgalloc.go +++ b/pkg/sentry/pgalloc/pgalloc.go @@ -209,6 +209,9 @@ type MemoryFileOpts struct { // If DisableIMAWorkAround is true, NewMemoryFile will not call // IMAWorkAroundForMemFile(). DisableIMAWorkAround bool + + // DiskBackedFile indicates that the MemoryFile is backed by a file on disk. + DiskBackedFile bool } // DelayedEvictionType is the type of MemoryFileOpts.DelayedEviction. @@ -580,35 +583,77 @@ func findAvailableRangeBottomUp(usage *usageSet, length, alignment uint64) (memm panic(fmt.Sprintf("NextLargeEnoughGap didn't return a gap at the end, length: %d", length)) } +// AllocationMode provides a way to inform the pgalloc API how to allocate +// memory and pages on the host. +// A page will exist in one of the following incremental states: +// 1. Allocated: A page is allocated if it was returned by Allocate() and its +// reference count hasn't dropped to 0 since then. +// 2. Committed: As described in MemoryFile documentation above, a page is +// committed if the host kernel is spending resources to store its +// contents. A committed page is implicitly allocated. +// 3. Populated: A page is populated for reading/writing in a page table +// hierarchy if it has a page table entry that permits reading/writing +// respectively. A populated page is implicitly committed, since the page +// table entry needs a physical page to point to, but not vice versa. +type AllocationMode int + +const ( + // AllocateOnly indicates that pages need to only be allocated. + AllocateOnly AllocationMode = iota + // AllocateAndCommit indicates that pages need to be committed, in addition + // to being allocated. + AllocateAndCommit + // AllocateAndWritePopulate indicates that writable pages should ideally be + // populated in the page table, in addition to being allocated. This is a + // suggestion, not a requirement. + AllocateAndWritePopulate +) + // AllocateAndFill allocates memory of the given kind and fills it by calling // r.ReadToBlocks() repeatedly until either length bytes are read or a non-nil // error is returned. It returns the memory filled by r, truncated down to the // nearest page. If this is shorter than length bytes due to an error returned // by r.ReadToBlocks(), it returns that error. // -// If populate is true, AllocateAndFill will attempt to pre-fault pages in bulk -// in the safemem.BlockSeq passed to r. Callers that will fill the allocated -// memory by writing to it in the sentry should pass populate = true to avoid -// faulting page-by-page. Callers that will fill the allocated memory by -// invoking host system calls should pass populate = false. +// allocMode allows the callers to select how the pages are allocated in the +// MemoryFile. Callers that will fill the allocated memory by writing to it +// should pass AllocateAndWritePopulate to avoid faulting page-by-page. Callers +// that will fill the allocated memory by invoking host system calls should +// pass AllocateOnly. Note that the mode may be upgraded in certain scenarios +// for performance. See implementation for more details. // // Preconditions: // - length > 0. // - length must be page-aligned. -func (f *MemoryFile) AllocateAndFill(length uint64, kind usage.MemoryKind, populate bool, r safemem.Reader) (memmap.FileRange, error) { +func (f *MemoryFile) AllocateAndFill(length uint64, kind usage.MemoryKind, allocMode AllocationMode, r safemem.Reader) (memmap.FileRange, error) { + if !f.opts.DiskBackedFile && allocMode == AllocateAndCommit { + // Upgrade to AllocateAndWritePopulate for memory(shmem)-backed files. We + // take a more aggressive approach in populating pages for memory-backed + // MemoryFiles. shmem pages are subject to swap rather than disk writeback. + // They are not likely to be swapped before they are written to. Hence it + // is beneficial to populate (in addition to commit) shmem pages to avoid + // faulting page-by-page when these pages are written to in the future. + allocMode = AllocateAndWritePopulate + } fr, err := f.Allocate(length, AllocOpts{Kind: kind}) if err != nil { return memmap.FileRange{}, err } + if allocMode == AllocateAndCommit { + if err := f.commitFile(fr); err != nil { + f.DecRef(fr) + return memmap.FileRange{}, err + } + } dsts, err := f.MapInternal(fr, hostarch.Write) if err != nil { f.DecRef(fr) return memmap.FileRange{}, err } - if populate && canPopulate() { + if allocMode == AllocateAndWritePopulate && canPopulate() { rem := dsts for { - if !tryPopulate(rem.Head()) { + if !f.tryPopulate(rem.Head()) { break } rem = rem.Tail() @@ -634,7 +679,11 @@ func canPopulate() bool { return mlockDisabled.Load() == 0 } -func tryPopulate(b safemem.Block) bool { +func (f *MemoryFile) tryPopulate(b safemem.Block) bool { + // For disk-backed MemoryFiles, mlock+munlock will populate pages read-only. + if mlockDisabled.Load() != 0 || f.opts.DiskBackedFile { + return false + } // Call mlock to populate pages, then munlock to cancel the mlock (but keep // the pages populated). Only do so for hugepage-aligned address ranges to // ensure that splitting the VMA in mlock doesn't split any existing @@ -705,6 +754,16 @@ func (f *MemoryFile) manuallyZero(fr memmap.FileRange) error { }) } +func (f *MemoryFile) commitFile(fr memmap.FileRange) error { + // "The default operation (i.e., mode is zero) of fallocate() allocates the + // disk space within the range specified by offset and len." - fallocate(2) + return unix.Fallocate( + int(f.file.Fd()), + 0, // mode + int64(fr.Start), + int64(fr.Length())) +} + func (f *MemoryFile) decommitFile(fr memmap.FileRange) error { // "After a successful call, subsequent reads from this range will // return zeroes. The FALLOC_FL_PUNCH_HOLE flag must be ORed with