diff --git a/pkg/sentry/contexttest/contexttest.go b/pkg/sentry/contexttest/contexttest.go index 46caaebc5..4ed00774f 100644 --- a/pkg/sentry/contexttest/contexttest.go +++ b/pkg/sentry/contexttest/contexttest.go @@ -116,8 +116,6 @@ func (t *TestContext) Value(key any) any { return t.l case pgalloc.CtxMemoryFile: return t.mf - case pgalloc.CtxMemoryFileProvider: - return t case platform.CtxPlatform: return t.platform case uniqueid.CtxGlobalUniqueID: @@ -136,11 +134,6 @@ func (t *TestContext) Value(key any) any { } } -// MemoryFile implements pgalloc.MemoryFileProvider.MemoryFile. -func (t *TestContext) MemoryFile() *pgalloc.MemoryFile { - return t.mf -} - // RootContext returns a Context that may be used in tests that need root // credentials. Uses ptrace as the platform.Platform. func RootContext(tb testing.TB) context.Context { diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 103daabc3..917188abb 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -183,9 +183,9 @@ type FilesystemType struct{} type filesystem struct { vfsfs vfs.Filesystem - // mfp is used to allocate memory that caches regular file contents. mfp is + // mf is used to allocate memory that caches regular file contents. mf is // immutable. - mfp pgalloc.MemoryFileProvider + mf *pgalloc.MemoryFile `state:"nosave"` // Immutable options. opts filesystemOptions @@ -395,9 +395,9 @@ func (FilesystemType) Release(ctx context.Context) {} // GetFilesystem implements vfs.FilesystemType.GetFilesystem. func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth.Credentials, source string, opts vfs.GetFilesystemOptions) (*vfs.Filesystem, *vfs.Dentry, error) { - mfp := pgalloc.MemoryFileProviderFromContext(ctx) - if mfp == nil { - ctx.Warningf("gofer.FilesystemType.GetFilesystem: context does not provide a pgalloc.MemoryFileProvider") + mf := pgalloc.MemoryFileFromContext(ctx) + if mf == nil { + ctx.Warningf("gofer.FilesystemType.GetFilesystem: CtxMemoryFile is nil") return nil, nil, linuxerr.EINVAL } @@ -522,7 +522,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, err } fs := &filesystem{ - mfp: mfp, + mf: mf, opts: fsopts, iopts: iopts, clock: ktime.RealtimeClockFromContext(ctx), @@ -660,7 +660,7 @@ func getFDFromMountOptionsMap(ctx context.Context, mopts map[string]string) (int func (fs *filesystem) Release(ctx context.Context) { fs.released.Store(1) - mf := fs.mfp.MemoryFile() + mf := fs.mf fs.syncMu.Lock() for elem := fs.syncableDentries.Front(); elem != nil; elem = elem.Next() { d := elem.d @@ -1385,7 +1385,7 @@ func (d *dentry) updateSizeAndUnlockDataMuLocked(newSize uint64) { // truncated pages have been removed from the remote file, they // should be dropped without being written back. d.dataMu.Lock() - d.cache.Truncate(newSize, d.fs.mfp.MemoryFile()) + d.cache.Truncate(newSize, d.fs.mf) d.dirty.KeepClean(memmap.MappableRange{newSize, oldpgend}) d.dataMu.Unlock() } @@ -1758,7 +1758,7 @@ func (d *dentry) evictLocked(ctx context.Context) { // destroyDisconnected destroys an uncached, unparented dentry. There are no // locking preconditions. func (d *dentry) destroyDisconnected(ctx context.Context) { - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf d.handleMu.Lock() d.dataMu.Lock() @@ -2080,7 +2080,7 @@ func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) err // Write back dirty pages to the remote file. d.dataMu.Lock() h := d.writeHandle() - err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) + err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), d.fs.mf, h.writeFromBlocksAt) d.dataMu.Unlock() if err != nil { return err diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index 90e0319e3..60e5097f1 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/pkg/sentry/fsimpl/gofer/gofer_test.go @@ -27,7 +27,7 @@ import ( func TestDestroyIdempotent(t *testing.T) { ctx := contexttest.Context(t) fs := filesystem{ - mfp: pgalloc.MemoryFileProviderFromContext(ctx), + mf: pgalloc.MemoryFileFromContext(ctx), inoByKey: make(map[inoKey]uint64), clock: time.RealtimeClockFromContext(ctx), // Test relies on no dentry being held in the cache. diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go index ad1bd7ae3..6c1ab5179 100644 --- a/pkg/sentry/fsimpl/gofer/regular_file.go +++ b/pkg/sentry/fsimpl/gofer/regular_file.go @@ -309,7 +309,7 @@ func (fd *regularFileFD) writeCache(ctx context.Context, d *dentry, offset int64 d.mapsMu.Unlock() // Finally free pages removed from the cache. - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf for _, freedFR := range freed { mf.DecRef(freedFR) } @@ -374,7 +374,7 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) } // Otherwise read from/through the cache. - mf := rw.d.fs.mfp.MemoryFile() + mf := rw.d.fs.mf fillCache := mf.ShouldCacheEvictable() var dataMuUnlock func() if fillCache { @@ -501,7 +501,7 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro } // Otherwise write to/through the cache. - mf := rw.d.fs.mfp.MemoryFile() + mf := rw.d.fs.mf rw.d.dataMu.Lock() // Compute the range to write (overflow-checked). @@ -608,7 +608,7 @@ func (d *dentry) writeback(ctx context.Context, offset, size int64) error { return fsutil.SyncDirty(ctx, memmap.MappableRange{ Start: uint64(offset), End: uint64(end), - }, &d.cache, &d.dirty, dentrySize, d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) + }, &d.cache, &d.dirty, dentrySize, d.fs.mf, h.writeFromBlocksAt) } // Seek implements vfs.FileDescriptionImpl.Seek. @@ -716,7 +716,7 @@ func (d *dentry) AddMapping(ctx context.Context, ms memmap.MappingSpace, ar host if d.fs.mayCachePagesInMemoryFile() { // d.Evict() will refuse to evict memory-mapped pages, so tell the // MemoryFile to not bother trying. - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf for _, r := range mapped { mf.MarkUnevictable(d, pgalloc.EvictableRange{r.Start, r.End}) } @@ -736,7 +736,7 @@ func (d *dentry) RemoveMapping(ctx context.Context, ms memmap.MappingSpace, ar h // Pages that are no longer referenced by any application memory // mappings are now considered unused; allow MemoryFile to evict them // when necessary. - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf d.dataMu.Lock() for _, r := range unmapped { // Since these pages are no longer mapped, they are no longer @@ -792,7 +792,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab optional.End = pgend } - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf h := d.readHandle() _, cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, pgalloc.AllocateAndWritePopulate, h.readToBlocksAt) @@ -862,7 +862,7 @@ func (d *dentry) InvalidateUnsavable(ctx context.Context) error { // Write the cache's contents back to the remote file so that if we have a // host fd after restore, the remote file's contents are coherent. - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf d.handleMu.RLock() defer d.handleMu.RUnlock() h := d.writeHandle() @@ -884,7 +884,7 @@ func (d *dentry) InvalidateUnsavable(ctx context.Context) error { // Evict implements pgalloc.EvictableMemoryUser.Evict. func (d *dentry) Evict(ctx context.Context, er pgalloc.EvictableRange) { mr := memmap.MappableRange{er.Start, er.End} - mf := d.fs.mfp.MemoryFile() + mf := d.fs.mf d.mapsMu.Lock() defer d.mapsMu.Unlock() d.handleMu.RLock() diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index ea537eb28..e16e11e7e 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -27,6 +27,7 @@ import ( "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/safemem" + "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/vfs" ) @@ -133,6 +134,11 @@ func (d *dentry) beforeSave() { } } +// afterLoad is invoked by stateify. +func (fs *filesystem) afterLoad(ctx goContext.Context) { + fs.mf = pgalloc.MemoryFileFromContext(ctx) +} + // afterLoad is invoked by stateify. func (d *dentry) afterLoad(goContext.Context) { d.readFD = atomicbitops.FromInt32(-1) diff --git a/pkg/sentry/fsimpl/iouringfs/iouringfs.go b/pkg/sentry/fsimpl/iouringfs/iouringfs.go index 635cf764a..f7ecebdda 100644 --- a/pkg/sentry/fsimpl/iouringfs/iouringfs.go +++ b/pkg/sentry/fsimpl/iouringfs/iouringfs.go @@ -50,7 +50,7 @@ type FileDescription struct { vfs.DentryMetadataFileDescriptionImpl vfs.NoLockFD - mfp pgalloc.MemoryFileProvider + mf *pgalloc.MemoryFile `state:"nosave"` rbmf ringsBufferFile sqemf sqEntriesFile @@ -95,9 +95,9 @@ func New(ctx context.Context, vfsObj *vfs.VirtualFilesystem, entries uint32, par vd := vfsObj.NewAnonVirtualDentry("[io_uring]") defer vd.DecRef(ctx) - mfp := pgalloc.MemoryFileProviderFromContext(ctx) - if mfp == nil { - panic(fmt.Sprintf("context.Context %T lacks non-nil value for key %T", ctx, pgalloc.CtxMemoryFileProvider)) + mf := pgalloc.MemoryFileFromContext(ctx) + if mf == nil { + panic(fmt.Sprintf("context.Context %T lacks non-nil value for key %T", ctx, pgalloc.CtxMemoryFile)) } numSqEntries, ok := roundUpPowerOfTwo(entries) @@ -123,7 +123,6 @@ func New(ctx context.Context, vfsObj *vfs.VirtualFilesystem, entries uint32, par numSqEntries*uint32((*linux.IORingIndex)(nil).SizeBytes())) ringsBufferSize = uint64(hostarch.Addr(ringsBufferSize).MustRoundUp()) - mf := mfp.MemoryFile() memCgID := pgalloc.MemoryCgroupIDFromContext(ctx) rbfr, err := mf.Allocate(ringsBufferSize, pgalloc.AllocOpts{Kind: usage.Anonymous, MemCgID: memCgID}) if err != nil { @@ -139,7 +138,7 @@ func New(ctx context.Context, vfsObj *vfs.VirtualFilesystem, entries uint32, par } iouringfd := &FileDescription{ - mfp: mfp, + mf: mf, rbmf: ringsBufferFile{ fr: rbfr, }, @@ -215,18 +214,15 @@ func New(ctx context.Context, vfsObj *vfs.VirtualFilesystem, entries uint32, par // Release implements vfs.FileDescriptionImpl.Release. func (fd *FileDescription) Release(ctx context.Context) { - mf := pgalloc.MemoryFileProviderFromContext(ctx).MemoryFile() - mf.DecRef(fd.rbmf.fr) - mf.DecRef(fd.sqemf.fr) + fd.mf.DecRef(fd.rbmf.fr) + fd.mf.DecRef(fd.sqemf.fr) } // mapSharedBuffers caches internal mappings for the ring's shared memory // regions. func (fd *FileDescription) mapSharedBuffers() error { - mf := fd.mfp.MemoryFile() - // Mapping for the IORings header struct. - rb, err := mf.MapInternal(fd.rbmf.fr, hostarch.ReadWrite) + rb, err := fd.mf.MapInternal(fd.rbmf.fr, hostarch.ReadWrite) if err != nil { return err } @@ -242,7 +238,7 @@ func (fd *FileDescription) mapSharedBuffers() error { fd.cqesBuf.init(cqes) // Mapping for the SQEs array. - sqes, err := mf.MapInternal(fd.sqemf.fr, hostarch.ReadWrite) + sqes, err := fd.mf.MapInternal(fd.sqemf.fr, hostarch.ReadWrite) if err != nil { return err } @@ -572,7 +568,7 @@ func (sqemf *sqEntriesFile) Translate(ctx context.Context, required, optional me return []memmap.Translation{ { Source: source, - File: pgalloc.MemoryFileProviderFromContext(ctx).MemoryFile(), + File: pgalloc.MemoryFileFromContext(ctx), Offset: sqemf.fr.Start + source.Start, Perms: at, }, @@ -618,7 +614,7 @@ func (rbmf *ringsBufferFile) Translate(ctx context.Context, required, optional m return []memmap.Translation{ { Source: source, - File: pgalloc.MemoryFileProviderFromContext(ctx).MemoryFile(), + File: pgalloc.MemoryFileFromContext(ctx), Offset: rbmf.fr.Start + source.Start, Perms: at, }, diff --git a/pkg/sentry/fsimpl/iouringfs/iouringfs_state.go b/pkg/sentry/fsimpl/iouringfs/iouringfs_state.go index 4a57dcb82..26ed5d210 100644 --- a/pkg/sentry/fsimpl/iouringfs/iouringfs_state.go +++ b/pkg/sentry/fsimpl/iouringfs/iouringfs_state.go @@ -14,7 +14,11 @@ package iouringfs -import "context" +import ( + "context" + + "gvisor.dev/gvisor/pkg/sentry/pgalloc" +) // beforeSave is invoked by stateify. func (fd *FileDescription) beforeSave() { @@ -24,7 +28,8 @@ func (fd *FileDescription) beforeSave() { } // afterLoad is invoked by stateify. -func (fd *FileDescription) afterLoad(context.Context) { +func (fd *FileDescription) afterLoad(ctx context.Context) { + fd.mf = pgalloc.MemoryFileFromContext(ctx) // Remap shared buffers. fd.remap = true fd.runC = make(chan struct{}, 1) diff --git a/pkg/sentry/fsimpl/testutil/kernel.go b/pkg/sentry/fsimpl/testutil/kernel.go index 3c9db90d5..5cf98481f 100644 --- a/pkg/sentry/fsimpl/testutil/kernel.go +++ b/pkg/sentry/fsimpl/testutil/kernel.go @@ -77,13 +77,13 @@ func Boot() (*kernel.Kernel, error) { k.SetMemoryFile(mf) // Pass k as the platform since it is savable, unlike the actual platform. - vdso, err := loader.PrepareVDSO(k) + vdso, err := loader.PrepareVDSO(k.MemoryFile()) if err != nil { return nil, fmt.Errorf("creating vdso: %v", err) } // Create timekeeper. - tk := kernel.NewTimekeeper(k, vdso.ParamPage.FileRange()) + tk := kernel.NewTimekeeper(k.MemoryFile(), vdso.ParamPage.FileRange()) tk.SetClocks(time.NewCalibratedClocks()) creds := auth.NewRootCredentials(auth.NewRootUserNamespace()) @@ -129,7 +129,7 @@ func CreateTask(ctx context.Context, name string, tc *kernel.ThreadGroup, mntns if err != nil { return nil, err } - m := mm.NewMemoryManager(k, k, k.SleepForAddressSpaceActivation) + m := mm.NewMemoryManager(k, k.MemoryFile(), k.SleepForAddressSpaceActivation) m.SetExecutable(ctx, exe) creds := auth.CredentialsFromContext(ctx) diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index f0996b7ec..5c698b5cb 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -134,7 +134,7 @@ type FilesystemOpts struct { MaxFilenameLen int // MemoryFile is the memory file that will be used to store file data. If - // this is nil, then MemoryFileProviderFromContext() is used. + // this is nil, then MemoryFileFromContext() is used. MemoryFile *pgalloc.MemoryFile // DisableDefaultSizeLimit disables setting a default size limit. In Linux, diff --git a/pkg/sentry/kernel/kcov.go b/pkg/sentry/kernel/kcov.go index 19b81ad7d..a1a198833 100644 --- a/pkg/sentry/kernel/kcov.go +++ b/pkg/sentry/kernel/kcov.go @@ -41,8 +41,8 @@ const kcovAreaSizeMax = 10 * 1024 * 1024 // To give the illusion that the data is always up to date, we update the shared // memory every time before we return to userspace. type Kcov struct { - // mfp provides application memory. It is immutable after creation. - mfp pgalloc.MemoryFileProvider + // mf stores application memory. It is immutable after creation. + mf *pgalloc.MemoryFile // mu protects all of the fields below. mu sync.RWMutex @@ -74,7 +74,7 @@ type Kcov struct { // NewKcov creates and returns a Kcov instance. func (k *Kernel) NewKcov() *Kcov { return &Kcov{ - mfp: k, + mf: k.mf, } } @@ -94,7 +94,7 @@ func (kcov *Kcov) TaskWork(t *Task) { } rw := &kcovReadWriter{ - mf: kcov.mfp.MemoryFile(), + mf: kcov.mf, fr: kcov.mappable.FileRange(), } @@ -246,7 +246,7 @@ func (kcov *Kcov) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) erro Kind: usage.Anonymous, MemCgID: pgalloc.MemoryCgroupIDFromContext(ctx), } - fr, err := kcov.mfp.MemoryFile().Allocate(kcov.size*8, opts) + fr, err := kcov.mf.Allocate(kcov.size*8, opts) if err != nil { return err } @@ -258,7 +258,7 @@ func (kcov *Kcov) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) erro } // For convenience, a special mappable is used here. Note that these mappings // will look different under /proc/[pid]/maps than they do on Linux. - kcov.mappable = mm.NewSpecialMappable(fmt.Sprintf("[kcov:%d]", t.ThreadID()), kcov.mfp, fr) + kcov.mappable = mm.NewSpecialMappable(fmt.Sprintf("[kcov:%d]", t.ThreadID()), kcov.mf, fr) } kcov.mappable.IncRef() opts.Mappable = kcov.mappable diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 4b49f0511..d0109772c 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -142,8 +142,7 @@ type Kernel struct { // All of the following fields are immutable unless otherwise specified. // Platform is the platform that is used to execute tasks in the created - // Kernel. See comment on pgalloc.MemoryFileProvider for why Platform is - // embedded anonymously (the same issue applies). + // Kernel. platform.Platform `state:"nosave"` // mf provides application memory. @@ -894,8 +893,6 @@ func (ctx *createProcessContext) Value(key any) any { return ctx.getMemoryCgroupID() case pgalloc.CtxMemoryFile: return ctx.kernel.mf - case pgalloc.CtxMemoryFileProvider: - return ctx.kernel case platform.CtxPlatform: return ctx.kernel case uniqueid.CtxGlobalUniqueID: @@ -1533,7 +1530,7 @@ func (k *Kernel) SetMemoryFile(mf *pgalloc.MemoryFile) { k.mf = mf } -// MemoryFile implements pgalloc.MemoryFileProvider.MemoryFile. +// MemoryFile returns the MemoryFile that provides application memory. func (k *Kernel) MemoryFile() *pgalloc.MemoryFile { return k.mf } @@ -1671,8 +1668,6 @@ func (ctx *supervisorContext) Value(key any) any { return limits.NewLimitSet() case pgalloc.CtxMemoryFile: return ctx.Kernel.mf - case pgalloc.CtxMemoryFileProvider: - return ctx.Kernel case platform.CtxPlatform: return ctx.Kernel case uniqueid.CtxGlobalUniqueID: diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go index 847680ffb..c6e9c2440 100644 --- a/pkg/sentry/kernel/shm/shm.go +++ b/pkg/sentry/kernel/shm/shm.go @@ -34,6 +34,7 @@ package shm import ( + goContext "context" "fmt" "gvisor.dev/gvisor/pkg/abi/linux" @@ -201,9 +202,9 @@ func (r *Registry) FindOrCreate(ctx context.Context, pid int32, key ipc.Key, siz // // Precondition: Caller must hold r.mu. func (r *Registry) newShmLocked(ctx context.Context, pid int32, key ipc.Key, creator *auth.Credentials, mode linux.FileMode, size uint64) (*Shm, error) { - mfp := pgalloc.MemoryFileProviderFromContext(ctx) - if mfp == nil { - panic(fmt.Sprintf("context.Context %T lacks non-nil value for key %T", ctx, pgalloc.CtxMemoryFileProvider)) + mf := pgalloc.MemoryFileFromContext(ctx) + if mf == nil { + panic(fmt.Sprintf("context.Context %T lacks non-nil value for key %T", ctx, pgalloc.CtxMemoryFile)) } devID, ok := deviceIDFromContext(ctx) if !ok { @@ -211,13 +212,13 @@ func (r *Registry) newShmLocked(ctx context.Context, pid int32, key ipc.Key, cre } effectiveSize := uint64(hostarch.Addr(size).MustRoundUp()) - fr, err := mfp.MemoryFile().Allocate(effectiveSize, pgalloc.AllocOpts{Kind: usage.Anonymous, MemCgID: pgalloc.MemoryCgroupIDFromContext(ctx)}) + fr, err := mf.Allocate(effectiveSize, pgalloc.AllocOpts{Kind: usage.Anonymous, MemCgID: pgalloc.MemoryCgroupIDFromContext(ctx)}) if err != nil { return nil, err } shm := &Shm{ - mfp: mfp, + mf: mf, registry: r, devID: devID, size: size, @@ -331,7 +332,7 @@ type Shm struct { // via MappingIdentity. ShmRefs - mfp pgalloc.MemoryFileProvider + mf *pgalloc.MemoryFile `state:"nosave"` // registry points to the shm registry containing this segment. Immutable. registry *Registry @@ -379,6 +380,11 @@ type Shm struct { pendingDestruction bool } +// afterLoad is invoked by stateify. +func (s *Shm) afterLoad(ctx goContext.Context) { + s.mf = pgalloc.MemoryFileFromContext(ctx) +} + // ID returns object's ID. func (s *Shm) ID() ipc.ID { return s.obj.ID @@ -436,7 +442,7 @@ func (s *Shm) InodeID() uint64 { // Precondition: Caller must not hold s.mu. func (s *Shm) DecRef(ctx context.Context) { s.ShmRefs.DecRef(func() { - s.mfp.MemoryFile().DecRef(s.fr) + s.mf.DecRef(s.fr) s.registry.remove(s) }) } @@ -498,7 +504,7 @@ func (s *Shm) Translate(ctx context.Context, required, optional memmap.MappableR return []memmap.Translation{ { Source: source, - File: s.mfp.MemoryFile(), + File: s.mf, Offset: s.fr.Start + source.Start, Perms: hostarch.AnyAccess, }, diff --git a/pkg/sentry/kernel/task_context.go b/pkg/sentry/kernel/task_context.go index 1e391c281..b1c1f667a 100644 --- a/pkg/sentry/kernel/task_context.go +++ b/pkg/sentry/kernel/task_context.go @@ -120,8 +120,6 @@ func (t *Task) contextValue(key any, isTaskGoroutine bool) any { return t.memCgID.Load() case pgalloc.CtxMemoryFile: return t.k.mf - case pgalloc.CtxMemoryFileProvider: - return t.k case platform.CtxPlatform: return t.k case shm.CtxDeviceID: diff --git a/pkg/sentry/kernel/task_image.go b/pkg/sentry/kernel/task_image.go index 18b674b72..e7fcffedd 100644 --- a/pkg/sentry/kernel/task_image.go +++ b/pkg/sentry/kernel/task_image.go @@ -146,7 +146,7 @@ func (t *Task) Stack() *arch.Stack { // args.MemoryManager does not need to be set by the caller. func (k *Kernel) LoadTaskImage(ctx context.Context, args loader.LoadArgs) (*TaskImage, *syserr.Error) { // Prepare a new user address space to load into. - m := mm.NewMemoryManager(k, k, k.SleepForAddressSpaceActivation) + m := mm.NewMemoryManager(k, k.mf, k.SleepForAddressSpaceActivation) defer m.DecUsers(ctx) args.MemoryManager = m diff --git a/pkg/sentry/kernel/timekeeper.go b/pkg/sentry/kernel/timekeeper.go index 5560b48d2..285e52543 100644 --- a/pkg/sentry/kernel/timekeeper.go +++ b/pkg/sentry/kernel/timekeeper.go @@ -97,9 +97,9 @@ type Timekeeper struct { // NewTimekeeper does not take ownership of paramPage. // // SetClocks must be called on the returned Timekeeper before it is usable. -func NewTimekeeper(mfp pgalloc.MemoryFileProvider, paramPage memmap.FileRange) *Timekeeper { +func NewTimekeeper(mf *pgalloc.MemoryFile, paramPage memmap.FileRange) *Timekeeper { t := Timekeeper{ - params: NewVDSOParamPage(mfp, paramPage), + params: NewVDSOParamPage(mf, paramPage), } t.realtimeClock = &timekeeperClock{tk: &t, c: sentrytime.Realtime} t.monotonicClock = &timekeeperClock{tk: &t, c: sentrytime.Monotonic} diff --git a/pkg/sentry/kernel/timekeeper_test.go b/pkg/sentry/kernel/timekeeper_test.go index 18358561d..e8720566f 100644 --- a/pkg/sentry/kernel/timekeeper_test.go +++ b/pkg/sentry/kernel/timekeeper_test.go @@ -53,13 +53,13 @@ func (c *mockClocks) GetTime(id sentrytime.ClockID) (int64, error) { // SetClocks called. func stateTestClocklessTimekeeper(tb testing.TB) *Timekeeper { ctx := contexttest.Context(tb) - mfp := pgalloc.MemoryFileProviderFromContext(ctx) - fr, err := mfp.MemoryFile().Allocate(hostarch.PageSize, pgalloc.AllocOpts{Kind: usage.Anonymous}) + mf := pgalloc.MemoryFileFromContext(ctx) + fr, err := mf.Allocate(hostarch.PageSize, pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { tb.Fatalf("failed to allocate memory: %v", err) } return &Timekeeper{ - params: NewVDSOParamPage(mfp, fr), + params: NewVDSOParamPage(mf, fr), } } diff --git a/pkg/sentry/kernel/vdso.go b/pkg/sentry/kernel/vdso.go index 7011f60fd..0e2f5fd38 100644 --- a/pkg/sentry/kernel/vdso.go +++ b/pkg/sentry/kernel/vdso.go @@ -15,6 +15,7 @@ package kernel import ( + "context" "fmt" "gvisor.dev/gvisor/pkg/hostarch" @@ -57,9 +58,9 @@ type vdsoParams struct { // // +stateify savable type VDSOParamPage struct { - // The parameter page is fr, allocated from mfp.MemoryFile(). - mfp pgalloc.MemoryFileProvider - fr memmap.FileRange + // The parameter page is fr, allocated from mf. + mf *pgalloc.MemoryFile `state:"nosave"` + fr memmap.FileRange // seq is the current sequence count written to the page. // @@ -78,17 +79,22 @@ type VDSOParamPage struct { copyScratchBuffer []byte } +// afterLoad is invoked by stateify. +func (v *VDSOParamPage) afterLoad(ctx context.Context) { + v.mf = pgalloc.MemoryFileFromContext(ctx) +} + // NewVDSOParamPage returns a VDSOParamPage. // // Preconditions: -// - fr is a single page allocated from mfp.MemoryFile(). VDSOParamPage does -// not take ownership of fr; it must remain allocated for the lifetime of the +// - fr is a single page allocated from mf. VDSOParamPage does not take +// ownership of fr; it must remain allocated for the lifetime of the // VDSOParamPage. // - VDSOParamPage must be the only writer to fr. -// - mfp.MemoryFile().MapInternal(fr) must return a single safemem.Block. -func NewVDSOParamPage(mfp pgalloc.MemoryFileProvider, fr memmap.FileRange) *VDSOParamPage { +// - mf.MapInternal(fr) must return a single safemem.Block. +func NewVDSOParamPage(mf *pgalloc.MemoryFile, fr memmap.FileRange) *VDSOParamPage { return &VDSOParamPage{ - mfp: mfp, + mf: mf, fr: fr, copyScratchBuffer: make([]byte, (*vdsoParams)(nil).SizeBytes()), } @@ -96,7 +102,7 @@ func NewVDSOParamPage(mfp pgalloc.MemoryFileProvider, fr memmap.FileRange) *VDSO // access returns a mapping of the param page. func (v *VDSOParamPage) access() (safemem.Block, error) { - bs, err := v.mfp.MemoryFile().MapInternal(v.fr, hostarch.ReadWrite) + bs, err := v.mf.MapInternal(v.fr, hostarch.ReadWrite) if err != nil { return safemem.Block{}, err } diff --git a/pkg/sentry/loader/vdso.go b/pkg/sentry/loader/vdso.go index 2f4a1477c..e3696f9b5 100644 --- a/pkg/sentry/loader/vdso.go +++ b/pkg/sentry/loader/vdso.go @@ -179,7 +179,7 @@ type VDSO struct { // PrepareVDSO validates the system VDSO and returns a VDSO, containing the // param page for updating by the kernel. -func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { +func PrepareVDSO(mf *pgalloc.MemoryFile) (*VDSO, error) { vdsoFile := &byteFullReader{data: vdsodata.Binary} // First make sure the VDSO is valid. vdsoFile does not use ctx, so a @@ -195,7 +195,6 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { return nil, fmt.Errorf("VDSO size overflows? %#x", len(vdsodata.Binary)) } - mf := mfp.MemoryFile() vdso, err := mf.Allocate(uint64(size), pgalloc.AllocOpts{Kind: usage.System}) if err != nil { return nil, fmt.Errorf("unable to allocate VDSO memory: %v", err) @@ -221,11 +220,11 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { } return &VDSO{ - ParamPage: mm.NewSpecialMappable("[vvar]", mfp, paramPage), + ParamPage: mm.NewSpecialMappable("[vvar]", mf, paramPage), // TODO(gvisor.dev/issue/157): Don't advertise the VDSO, as // some applications may not be able to handle multiple [vdso] // hints. - vdso: mm.NewSpecialMappable("", mfp, vdso), + vdso: mm.NewSpecialMappable("", mf, vdso), os: info.os, arch: info.arch, phdrs: info.phdrs, diff --git a/pkg/sentry/mm/aio_context.go b/pkg/sentry/mm/aio_context.go index f5210b48c..185b94e49 100644 --- a/pkg/sentry/mm/aio_context.go +++ b/pkg/sentry/mm/aio_context.go @@ -246,18 +246,18 @@ func (aio *AIOContext) Drain() { type aioMappable struct { aioMappableRefs - mfp pgalloc.MemoryFileProvider - fr memmap.FileRange + mf *pgalloc.MemoryFile `state:"nosave"` + fr memmap.FileRange } var aioRingBufferSize = uint64(hostarch.Addr(linux.AIORingSize).MustRoundUp()) -func newAIOMappable(ctx context.Context, mfp pgalloc.MemoryFileProvider) (*aioMappable, error) { - fr, err := mfp.MemoryFile().Allocate(aioRingBufferSize, pgalloc.AllocOpts{Kind: usage.Anonymous, MemCgID: pgalloc.MemoryCgroupIDFromContext(ctx)}) +func newAIOMappable(ctx context.Context, mf *pgalloc.MemoryFile) (*aioMappable, error) { + fr, err := mf.Allocate(aioRingBufferSize, pgalloc.AllocOpts{Kind: usage.Anonymous, MemCgID: pgalloc.MemoryCgroupIDFromContext(ctx)}) if err != nil { return nil, err } - m := aioMappable{mfp: mfp, fr: fr} + m := aioMappable{mf: mf, fr: fr} m.InitRefs() return &m, nil } @@ -265,7 +265,7 @@ func newAIOMappable(ctx context.Context, mfp pgalloc.MemoryFileProvider) (*aioMa // DecRef implements refs.RefCounter.DecRef. func (m *aioMappable) DecRef(ctx context.Context) { m.aioMappableRefs.DecRef(func() { - m.mfp.MemoryFile().DecRef(m.fr) + m.mf.DecRef(m.fr) }) } @@ -346,7 +346,7 @@ func (m *aioMappable) Translate(ctx context.Context, required, optional memmap.M return []memmap.Translation{ { Source: source, - File: m.mfp.MemoryFile(), + File: m.mf, Offset: m.fr.Start + source.Start, Perms: hostarch.AnyAccess, }, @@ -368,7 +368,7 @@ func (mm *MemoryManager) NewAIOContext(ctx context.Context, events uint32) (uint // libaio peeks inside looking for a magic number. This function allocates // a page per context and keeps it set to zeroes to ensure it will not // match AIO_RING_MAGIC and make libaio happy. - m, err := newAIOMappable(ctx, mm.mfp) + m, err := newAIOMappable(ctx, mm.mf) if err != nil { return 0, err } diff --git a/pkg/sentry/mm/aio_context_state.go b/pkg/sentry/mm/aio_context_state.go index 61ebe8af0..c60de6575 100644 --- a/pkg/sentry/mm/aio_context_state.go +++ b/pkg/sentry/mm/aio_context_state.go @@ -14,9 +14,18 @@ package mm -import "context" +import ( + "context" + + "gvisor.dev/gvisor/pkg/sentry/pgalloc" +) // afterLoad is invoked by stateify. func (aio *AIOContext) afterLoad(context.Context) { aio.requestReady = make(chan struct{}, 1) } + +// afterLoad is invoked by stateify. +func (m *aioMappable) afterLoad(ctx context.Context) { + m.mf = pgalloc.MemoryFileFromContext(ctx) +} diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index ae0d2d282..0b5a836fe 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -28,11 +28,10 @@ import ( ) // NewMemoryManager returns a new MemoryManager with no mappings and 1 user. -func NewMemoryManager(p platform.Platform, mfp pgalloc.MemoryFileProvider, sleepForActivation bool) *MemoryManager { +func NewMemoryManager(p platform.Platform, mf *pgalloc.MemoryFile, sleepForActivation bool) *MemoryManager { return &MemoryManager{ p: p, - mfp: mfp, - mf: mfp.MemoryFile(), + mf: mf, haveASIO: p.SupportsAddressSpaceIO(), users: atomicbitops.FromInt32(1), auxv: arch.Auxv{}, @@ -74,7 +73,6 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) { defer mm.mappingMu.RUnlock() mm2 := &MemoryManager{ p: mm.p, - mfp: mm.mfp, mf: mm.mf, haveASIO: mm.haveASIO, layout: mm.layout, diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go index 033430ad6..dc1431889 100644 --- a/pkg/sentry/mm/mm.go +++ b/pkg/sentry/mm/mm.go @@ -57,8 +57,7 @@ type MapsCallbackFunc func(start, end hostarch.Addr, permissions hostarch.Access // +stateify savable type MemoryManager struct { // p and mfp are immutable. - p platform.Platform - mfp pgalloc.MemoryFileProvider + p platform.Platform // mf is the cached result of mfp.MemoryFile(). // diff --git a/pkg/sentry/mm/mm_test.go b/pkg/sentry/mm/mm_test.go index 4fced2730..aafae4454 100644 --- a/pkg/sentry/mm/mm_test.go +++ b/pkg/sentry/mm/mm_test.go @@ -31,8 +31,7 @@ import ( func testMemoryManager(ctx context.Context) *MemoryManager { p := platform.FromContext(ctx) - mfp := pgalloc.MemoryFileProviderFromContext(ctx) - mm := NewMemoryManager(p, mfp, false) + mm := NewMemoryManager(p, pgalloc.MemoryFileFromContext(ctx), false) mm.layout = arch.MmapLayout{ MinAddr: p.MinUserAddress(), MaxAddr: p.MaxUserAddress(), diff --git a/pkg/sentry/mm/save_restore.go b/pkg/sentry/mm/save_restore.go index 8ca3e3c33..6f2e0ceb7 100644 --- a/pkg/sentry/mm/save_restore.go +++ b/pkg/sentry/mm/save_restore.go @@ -38,11 +38,16 @@ func (mm *MemoryManager) InvalidateUnsavable(ctx context.Context) error { } // afterLoad is invoked by stateify. -func (mm *MemoryManager) afterLoad(goContext.Context) { - mm.mf = mm.mfp.MemoryFile() +func (mm *MemoryManager) afterLoad(ctx goContext.Context) { + mm.mf = pgalloc.MemoryFileFromContext(ctx) mm.haveASIO = mm.p.SupportsAddressSpaceIO() } +// afterLoad is invoked by stateify. +func (m *SpecialMappable) afterLoad(ctx goContext.Context) { + m.mf = pgalloc.MemoryFileFromContext(ctx) +} + const ( vmaRealPermsRead = 1 << iota vmaRealPermsWrite diff --git a/pkg/sentry/mm/special_mappable.go b/pkg/sentry/mm/special_mappable.go index d0270aba2..fe1148ab9 100644 --- a/pkg/sentry/mm/special_mappable.go +++ b/pkg/sentry/mm/special_mappable.go @@ -31,7 +31,7 @@ import ( type SpecialMappable struct { SpecialMappableRefs - mfp pgalloc.MemoryFileProvider + mf *pgalloc.MemoryFile `state:"nosave"` fr memmap.FileRange name string } @@ -41,8 +41,8 @@ type SpecialMappable struct { // SpecialMappable will use the given name in /proc/[pid]/maps. // // Preconditions: fr.Length() != 0. -func NewSpecialMappable(name string, mfp pgalloc.MemoryFileProvider, fr memmap.FileRange) *SpecialMappable { - m := SpecialMappable{mfp: mfp, fr: fr, name: name} +func NewSpecialMappable(name string, mf *pgalloc.MemoryFile, fr memmap.FileRange) *SpecialMappable { + m := SpecialMappable{mf: mf, fr: fr, name: name} m.InitRefs() return &m } @@ -50,7 +50,7 @@ func NewSpecialMappable(name string, mfp pgalloc.MemoryFileProvider, fr memmap.F // DecRef implements refs.RefCounter.DecRef. func (m *SpecialMappable) DecRef(ctx context.Context) { m.SpecialMappableRefs.DecRef(func() { - m.mfp.MemoryFile().DecRef(m.fr) + m.mf.DecRef(m.fr) }) } @@ -99,7 +99,7 @@ func (m *SpecialMappable) Translate(ctx context.Context, required, optional memm return []memmap.Translation{ { Source: source, - File: m.mfp.MemoryFile(), + File: m.mf, Offset: m.fr.Start + source.Start, Perms: hostarch.AnyAccess, }, @@ -115,14 +115,8 @@ func (m *SpecialMappable) InvalidateUnsavable(ctx context.Context) error { return nil } -// MemoryFileProvider returns the MemoryFileProvider whose MemoryFile stores -// the SpecialMappable's contents. -func (m *SpecialMappable) MemoryFileProvider() pgalloc.MemoryFileProvider { - return m.mfp -} - -// FileRange returns the offsets into MemoryFileProvider().MemoryFile() that -// store the SpecialMappable's contents. +// FileRange returns the offsets into m.mf that stores the SpecialMappable's +// contents. func (m *SpecialMappable) FileRange() memmap.FileRange { return m.fr } diff --git a/pkg/sentry/pgalloc/context.go b/pkg/sentry/pgalloc/context.go index 94759f521..88c897f2f 100644 --- a/pkg/sentry/pgalloc/context.go +++ b/pkg/sentry/pgalloc/context.go @@ -25,9 +25,6 @@ const ( // CtxMemoryFile is a Context.Value key for a MemoryFile. CtxMemoryFile contextID = iota - // CtxMemoryFileProvider is a Context.Value key for a MemoryFileProvider. - CtxMemoryFileProvider - // CtxMemoryCgroupID is the memory cgroup id which the task belongs to. CtxMemoryCgroupID @@ -45,15 +42,6 @@ func MemoryFileFromContext(ctx context.Context) *MemoryFile { return nil } -// MemoryFileProviderFromContext returns the MemoryFileProvider used by ctx, or -// nil if no such MemoryFileProvider exists. -func MemoryFileProviderFromContext(ctx context.Context) MemoryFileProvider { - if v := ctx.Value(CtxMemoryFileProvider); v != nil { - return v.(MemoryFileProvider) - } - return nil -} - // MemoryCgroupIDFromContext returns the memory cgroup id of the ctx, or // zero if the ctx does not belong to any memory cgroup. func MemoryCgroupIDFromContext(ctx context.Context) uint32 { diff --git a/pkg/sentry/pgalloc/save_restore.go b/pkg/sentry/pgalloc/save_restore.go index c8cf27367..c9b3878b6 100644 --- a/pkg/sentry/pgalloc/save_restore.go +++ b/pkg/sentry/pgalloc/save_restore.go @@ -213,19 +213,3 @@ func (f *MemoryFile) LoadFrom(ctx context.Context, r wire.Reader) error { return nil } - -// MemoryFileProvider provides the MemoryFile method. -// -// This type exists to work around a save/restore defect. The only object in a -// saved object graph that S/R allows to be replaced at time of restore is the -// starting point of the restore, kernel.Kernel. However, the MemoryFile -// changes between save and restore as well, so objects that need persistent -// access to the MemoryFile must instead store a pointer to the Kernel and call -// Kernel.MemoryFile() as required. In most cases, depending on the kernel -// package directly would create a package dependency loop, so the stored -// pointer must instead be a MemoryProvider interface object. Correspondingly, -// kernel.Kernel is the only implementation of this interface. -type MemoryFileProvider interface { - // MemoryFile returns the Kernel MemoryFile. - MemoryFile() *MemoryFile -} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index b4b2c7948..b029a0dc3 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -424,13 +424,13 @@ func New(args Args) (*Loader, error) { // Create VDSO. // // Pass k as the platform since it is savable, unlike the actual platform. - vdso, err := loader.PrepareVDSO(k) + vdso, err := loader.PrepareVDSO(k.MemoryFile()) if err != nil { return nil, fmt.Errorf("creating vdso: %w", err) } // Create timekeeper. - tk := kernel.NewTimekeeper(k, vdso.ParamPage.FileRange()) + tk := kernel.NewTimekeeper(k.MemoryFile(), vdso.ParamPage.FileRange()) tk.SetClocks(time.NewCalibratedClocks()) if err := enableStrace(args.Conf); err != nil {