diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index 0e3122a6f..aa659e9c2 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -92,6 +92,7 @@ go_library( "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", + "//pkg/fd", "//pkg/fspath", "//pkg/hostarch", "//pkg/log", diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 543748ad1..49f3dc3ff 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -42,9 +42,6 @@ import ( type regularFile struct { inode inode - // memFile is a platform.File used to allocate pages to this regularFile. - memFile *pgalloc.MemoryFile `state:"nosave"` - // memoryUsageKind is the memory accounting category under which pages backing // this regularFile's contents are accounted. memoryUsageKind usage.MemoryKind @@ -93,7 +90,6 @@ type regularFile struct { func (fs *filesystem) newRegularFile(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, parentDir *directory) *inode { file := ®ularFile{ - memFile: fs.mfp.MemoryFile(), memoryUsageKind: fs.usage, seals: linux.F_SEAL_SEAL, } @@ -224,7 +220,7 @@ func (rf *regularFile) truncateLocked(newSize uint64) (bool, error) { // We are now guaranteed that there are no translations of truncated pages, // and can remove them. rf.dataMu.Lock() - decPages := rf.data.Truncate(newSize, rf.memFile) + decPages := rf.data.Truncate(newSize, rf.inode.fs.mf) rf.dataMu.Unlock() rf.inode.fs.unaccountPages(decPages) return true, nil @@ -311,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.memFile, 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, false /* populate */, 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 }) @@ -325,7 +321,7 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap. segMR := seg.Range().Intersect(optional) ts = append(ts, memmap.Translation{ Source: segMR, - File: rf.memFile, + File: rf.inode.fs.mf, Offset: seg.FileRangeOf(segMR).Start, Perms: hostarch.AnyAccess, }) @@ -392,7 +388,7 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint // 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.memFile, f.memoryUsageKind, true /* populate */, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { + 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) { // Newly-allocated pages are zeroed, so we don't need to do anything. return dsts.NumBytes(), nil }) @@ -606,7 +602,7 @@ func (rw *regularFileReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, er switch { case seg.Ok(): // Get internal mappings. - ims, err := rw.file.memFile.MapInternal(seg.FileRangeOf(seg.Range().Intersect(mr)), hostarch.Read) + ims, err := rw.file.inode.fs.mf.MapInternal(seg.FileRangeOf(seg.Range().Intersect(mr)), hostarch.Read) if err != nil { return done, err } @@ -700,7 +696,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, switch { case seg.Ok(): // Get internal mappings. - ims, err := rw.file.memFile.MapInternal(seg.FileRangeOf(seg.Range().Intersect(mr)), hostarch.Write) + ims, err := rw.file.inode.fs.mf.MapInternal(seg.FileRangeOf(seg.Range().Intersect(mr)), hostarch.Write) if err != nil { retErr = err goto exitLoop @@ -733,7 +729,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, goto exitLoop } gapMR.End = gapMR.Start + (hostarch.PageSize * pagesReserved) - fr, err := rw.file.memFile.Allocate(gapMR.Length(), pgalloc.AllocOpts{Kind: rw.file.memoryUsageKind}) + fr, err := rw.file.inode.fs.mf.Allocate(gapMR.Length(), pgalloc.AllocOpts{Kind: rw.file.memoryUsageKind}) if err != nil { retErr = err rw.file.inode.fs.unaccountPages(pagesReserved) diff --git a/pkg/sentry/fsimpl/tmpfs/save_restore.go b/pkg/sentry/fsimpl/tmpfs/save_restore.go index b27f75cc2..a540e8915 100644 --- a/pkg/sentry/fsimpl/tmpfs/save_restore.go +++ b/pkg/sentry/fsimpl/tmpfs/save_restore.go @@ -15,6 +15,10 @@ package tmpfs // afterLoad is called by stateify. -func (rf *regularFile) afterLoad() { - rf.memFile = rf.inode.fs.mfp.MemoryFile() +func (fs *filesystem) afterLoad() { + if fs.privateMF { + // TODO(b/241832602): Add S/R support. + panic("S/R not supported for private memory files") + } + fs.mf = fs.mfp.MemoryFile() } diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 9aabc06a0..d1412fd0d 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -38,6 +38,7 @@ import ( "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/time" @@ -61,8 +62,16 @@ type FilesystemType struct{} type filesystem struct { vfsfs vfs.Filesystem - // mfp is used to allocate memory that stores regular file contents. mfp is - // immutable. + // mf is used to allocate memory that stores regular file contents. mf is + // immutable, except it may to changed during restore. + mf *pgalloc.MemoryFile `state:"nosave"` + + // privateMF indicates whether mf is private to this tmpfs mount. If so, + // tmpfs takes ownership of mf. privateMF is immutable. + privateMF bool + + // mfp is used to provide mf, when privateMF == false. This is required to + // re-provide mf on restore. mfp is immutable. mfp pgalloc.MemoryFileProvider // clock is a realtime clock used to set timestamps in file operations. @@ -128,9 +137,9 @@ type FilesystemOpts struct { // MaxFilenameLen is the maximum filename length allowed by the tmpfs. MaxFilenameLen int - // Filestore is the MemoryFile that will be used to store file data. If this - // is nil, then MemoryFileProviderFromContext() is used. - Filestore *pgalloc.MemoryFile + // FilestoreFD is the FD for the memory file that will be used to store file + // data. If this is nil, then MemoryFileProviderFromContext() is used. + FilestoreFD *fd.FD } // GetFilesystem implements vfs.FilesystemType.GetFilesystem. @@ -139,6 +148,8 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt if mfp == nil { panic("MemoryFileProviderFromContext returned nil") } + mf := mfp.MemoryFile() + privateMF := false rootFileType := uint16(linux.S_IFDIR) newFSType := vfs.FilesystemType(&fstype) @@ -150,8 +161,20 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt if tmpfsOpts.FilesystemType != nil { newFSType = tmpfsOpts.FilesystemType } - if tmpfsOpts.Filestore != nil { - mfp = tmpfsOpts.Filestore + 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} + var err error + mf, err = pgalloc.NewMemoryFile(tmpfsOpts.FilestoreFD.ReleaseToFile("overlay-filestore"), mfOpts) + if err != nil { + ctx.Warningf("tmpfs.FilesystemType.GetFilesystem: pgalloc.NewMemoryFile failed: %v", err) + return nil, nil, err + } + privateMF = true } } @@ -243,6 +266,8 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt memUsage = *tmpfsOpts.Usage } fs := filesystem{ + mf: mf, + privateMF: privateMF, mfp: mfp, clock: clock, devMinor: devMinor, @@ -285,6 +310,9 @@ func (fs *filesystem) Release(ctx context.Context) { fs.root.releaseChildrenLocked(ctx) } fs.mu.Unlock() + if fs.privateMF { + fs.mf.Destroy() + } } // releaseChildrenLocked is called on the mount point by filesystem.Release() to @@ -533,7 +561,7 @@ func (i *inode) decRef(ctx context.Context) { // Release memory used by regFile to store data. Since regFile is // no longer usable, we don't need to grab any locks or update any // metadata. - pagesDec := impl.data.DropAll(impl.memFile) + pagesDec := impl.data.DropAll(i.fs.mf) impl.inode.fs.unaccountPages(pagesDec) } diff --git a/pkg/sentry/pgalloc/pgalloc.go b/pkg/sentry/pgalloc/pgalloc.go index 447bfd97f..3757fc3eb 100644 --- a/pkg/sentry/pgalloc/pgalloc.go +++ b/pkg/sentry/pgalloc/pgalloc.go @@ -205,6 +205,10 @@ type MemoryFileOpts struct { // obtained from the host are zero-filled, such that MemoryFile must manually // zero newly-allocated pages. ManualZeroing bool + + // If DisableIMAWorkAround is true, NewMemoryFile will not call + // IMAWorkAroundForMemFile(). + DisableIMAWorkAround bool } // DelayedEvictionType is the type of MemoryFileOpts.DelayedEviction. @@ -357,24 +361,31 @@ func NewMemoryFile(file *os.File, opts MemoryFileOpts) (*MemoryFile, error) { go f.runReclaim() // S/R-SAFE: f.mu - // The Linux kernel contains an optional feature called "Integrity - // Measurement Architecture" (IMA). If IMA is enabled, it will checksum - // binaries the first time they are mapped PROT_EXEC. This is bad news for - // executable pages mapped from our backing file, which can grow to - // terabytes in (sparse) size. If IMA attempts to checksum a file that - // large, it will allocate all of the sparse pages and quickly exhaust all - // memory. - // - // Work around IMA by immediately creating a temporary PROT_EXEC mapping, - // while the backing file is still small. IMA will ignore any future - // mappings. + if !opts.DisableIMAWorkAround { + IMAWorkAroundForMemFile(file.Fd()) + } + return f, nil +} + +// IMAWorkAroundForMemFile works around IMA by immediately creating a temporary +// PROT_EXEC mapping, while the backing file is still small. IMA will ignore +// any future mappings. +// +// The Linux kernel contains an optional feature called "Integrity +// Measurement Architecture" (IMA). If IMA is enabled, it will checksum +// binaries the first time they are mapped PROT_EXEC. This is bad news for +// executable pages mapped from our backing file, which can grow to +// terabytes in (sparse) size. If IMA attempts to checksum a file that +// large, it will allocate all of the sparse pages and quickly exhaust all +// memory. +func IMAWorkAroundForMemFile(fd uintptr) { m, _, errno := unix.Syscall6( unix.SYS_MMAP, 0, hostarch.PageSize, unix.PROT_EXEC, unix.MAP_SHARED, - file.Fd(), + fd, 0) if errno != 0 { // This isn't fatal (IMA may not even be in use). Log the error, but @@ -389,8 +400,6 @@ func NewMemoryFile(file *os.File, opts MemoryFileOpts) (*MemoryFile, error) { panic(fmt.Sprintf("failed to unmap PROT_EXEC MemoryFile mapping: %v", errno)) } } - - return f, nil } // Destroy releases all resources used by f. @@ -1364,11 +1373,6 @@ func (f *MemoryFile) startEvictionGoroutineLocked(user EvictableMemoryUser, info }() } -// MemoryFile implements MemoryFileProvider.MemoryFile. -func (f *MemoryFile) MemoryFile() *MemoryFile { - return f -} - // WaitForEvictions blocks until f is no longer evicting any evictable // allocations. func (f *MemoryFile) WaitForEvictions() { diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 7d5089add..f0babe326 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -92,10 +92,6 @@ const ( // ContMgrProcfsDump dumps sandbox procfs state. ContMgrProcfsDump = "containerManager.ProcfsDump" - - // CongMgrOverlayFileUsage returns the current usage (bytes) of the overlay - // filestore. - CongMgrOverlayFileUsage = "containerManager.OverlayFileUsage" ) const ( @@ -268,6 +264,7 @@ type StartArgs struct { // FilePayload contains, in order: // * stdin, stdout, and stderr (optional: if terminal is disabled). + // * file descriptor to overlay-backing host file (optional: for overlay2). // * file descriptors to connect to gofer to serve the root filesystem. urpc.FilePayload } @@ -288,8 +285,16 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro if args.CID == "" { return errors.New("start argument missing container ID") } - if len(args.Files) < 1 { - return fmt.Errorf("start arguments must contain at least one file for the container root gofer") + expectedFDs := 1 // At least one FD for the root filesystem. + if !args.Spec.Process.Terminal { + expectedFDs += 3 + } + overlay2 := args.Conf.GetOverlay2() + if overlay2.IsBackedByHostFile() { + expectedFDs++ + } + if len(args.Files) < expectedFDs { + return fmt.Errorf("start arguments must contain at least %d FDs, but only got %d", expectedFDs, len(args.Files)) } // All validation passed, logs the spec for debugging. @@ -300,9 +305,6 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro if !args.Spec.Process.Terminal { // When not using a terminal, stdios come as the first 3 files in the // payload. - if l := len(args.Files); l < 4 { - return fmt.Errorf("start arguments (len: %d) must contain stdios and files for the container root gofer", l) - } var err error stdios, err = fd.NewFromFiles(goferFiles[:3]) if err != nil { @@ -316,6 +318,16 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro } }() + var overlayFilestoreFD *fd.FD + if overlay2.IsBackedByHostFile() { + var err error + overlayFilestoreFD, err = fd.NewFromFile(goferFiles[0]) + if err != nil { + return fmt.Errorf("error dup'ing overlay filestore file: %w", err) + } + goferFiles = goferFiles[1:] + } + goferFDs, err := fd.NewFromFiles(goferFiles) if err != nil { return fmt.Errorf("error dup'ing gofer files: %w", err) @@ -326,7 +338,7 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro } }() - if err := cm.l.startSubcontainer(args.Spec, args.Conf, args.CID, stdios, goferFDs); err != nil { + if err := cm.l.startSubcontainer(args.Spec, args.Conf, args.CID, stdios, goferFDs, overlayFilestoreFD); err != nil { log.Debugf("containerManager.StartSubcontainer failed, cid: %s, args: %+v, err: %v", args.CID, args, err) return err } @@ -628,18 +640,3 @@ func (cm *containerManager) ProcfsDump(_ *struct{}, out *[]procfs.ProcessProcfsD } return nil } - -// OverlayFileUsage returns the current usage (bytes) of the overlay filestore. -func (cm *containerManager) OverlayFileUsage(_ *struct{}, out *uint64) error { - *out = 0 - if cm.l.root.overlayFilestore == nil { - return nil - } - - usage, err := cm.l.root.overlayFilestore.TotalUsage() - if err != nil { - return err - } - *out = usage - return nil -} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index d48bcb8e0..7e40f20be 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -101,9 +101,9 @@ type containerInfo struct { // goferFDs are the FDs that attach the sandbox to the gofers. goferFDs []*fd.FD - // overlayFilestore is the memory file that will back the overlay mount's - // upper tmpfs layer. - overlayFilestore *pgalloc.MemoryFile + // overlayFilestoreFD is the FD for the memory file that will back the + // overlay mount's upper tmpfs layer. + overlayFilestoreFD *fd.FD } // Loader keeps state needed to start the kernel and run the container. @@ -274,12 +274,7 @@ func New(args Args) (*Loader, error) { info.goferFDs = append(info.goferFDs, fd.New(goferFD)) } if args.OverlayFilestoreFD >= 0 { - f := os.NewFile(uintptr(args.OverlayFilestoreFD), "overlay-filestore") - mf, err := pgalloc.NewMemoryFile(f, pgalloc.MemoryFileOpts{DecommitOnDestroy: true}) - if err != nil { - return nil, fmt.Errorf("tmpfs.FilesystemType.GetFilesystem: failed to create memory file from host file: %w", err) - } - info.overlayFilestore = mf + info.overlayFilestoreFD = fd.New(args.OverlayFilestoreFD) } // Create kernel and platform. @@ -526,9 +521,6 @@ func (l *Loader) Destroy() { for _, f := range l.root.goferFDs { _ = f.Close() } - if l.root.overlayFilestore != nil { - l.root.overlayFilestore.Destroy() - } l.stopProfiling() } @@ -709,7 +701,7 @@ func (l *Loader) createSubcontainer(cid string, tty *fd.FD) error { // startSubcontainer starts a child container. It returns the thread group ID of // the newly created process. Used FDs are either closed or released. It's safe // for the caller to close any remaining files upon return. -func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdioFDs, goferFDs []*fd.FD) error { +func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdioFDs, goferFDs []*fd.FD, overlayFilestoreFD *fd.FD) error { // Create capabilities. caps, err := specutils.Capabilities(conf.EnableRaw, spec.Process.Capabilities) if err != nil { @@ -760,12 +752,10 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st } info := &containerInfo{ - conf: conf, - spec: spec, - goferFDs: goferFDs, - // Note that K8s starts all containers in a pod (root and subcontainers) - // with the same config. So overlayFilestore can be copied. - overlayFilestore: l.root.overlayFilestore, + conf: conf, + spec: spec, + goferFDs: goferFDs, + overlayFilestoreFD: overlayFilestoreFD, } info.procArgs, err = createProcessArgs(cid, spec, creds, l.k, pidns) if err != nil { diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 1a1d64de9..84458027d 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -46,7 +46,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/inet" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" - "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/runsc/config" "gvisor.dev/gvisor/runsc/specutils" @@ -316,9 +315,9 @@ type containerMounter struct { // fds is the list of FDs to be dispensed for mounts that require it. fds fdDispenser - // overlayFilestore is the memory file that will back the overlay mount's - // upper tmpfs layer. - overlayFilestore *pgalloc.MemoryFile + // overlayFilestoreFD is the FD for the memory file that will back the + // overlay mount's upper tmpfs layer. + overlayFilestoreFD *fd.FD k *kernel.Kernel @@ -331,13 +330,13 @@ type containerMounter struct { func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountHints, productName string) *containerMounter { return &containerMounter{ - root: info.spec.Root, - mounts: compileMounts(info.spec, info.conf), - fds: fdDispenser{fds: info.goferFDs}, - overlayFilestore: info.overlayFilestore, - k: k, - hints: hints, - productName: productName, + root: info.spec.Root, + mounts: compileMounts(info.spec, info.conf), + fds: fdDispenser{fds: info.goferFDs}, + overlayFilestoreFD: info.overlayFilestoreFD, + k: k, + hints: hints, + productName: productName, } } @@ -487,7 +486,7 @@ func (c *containerMounter) configureOverlay(ctx context.Context, creds *auth.Cre // Upper is a tmpfs mount to keep all modifications inside the sandbox. upperOpts.GetFilesystemOptions.InternalData = tmpfs.FilesystemOpts{ RootFileType: uint16(rootType), - Filestore: c.overlayFilestore, + FilestoreFD: c.overlayFilestoreFD, } upper, err := c.k.VFS().MountDisconnected(ctx, creds, "" /* source */, tmpfs.Name, &upperOpts) if err != nil { diff --git a/runsc/container/BUILD b/runsc/container/BUILD index d445ed7d3..8e454a928 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/cleanup", "//pkg/log", "//pkg/sentry/control", + "//pkg/sentry/pgalloc", "//pkg/sighandling", "//pkg/sync", "//runsc/boot", diff --git a/runsc/container/container.go b/runsc/container/container.go index c07b0fce7..a6fa7ad59 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -35,6 +35,7 @@ import ( "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/control" + "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sighandling" "gvisor.dev/gvisor/runsc/boot" "gvisor.dev/gvisor/runsc/cgroup" @@ -263,7 +264,7 @@ func New(conf *config.Config, args Args) (*Container, error) { } } c.CompatCgroup = cgroup.CgroupJSON{Cgroup: subCgroup} - overlayFilestoreFile, err := createOverlayFilestore(conf.GetOverlay2()) + overlayFilestoreFile, err := createOverlayFilestore(conf) if err != nil { return nil, err } @@ -400,6 +401,12 @@ func (c *Container) Start(conf *config.Config) error { return err } } else { + // Create an overlay filestore for the subcontainer if its overlay is + // backed by a host file. + overlayFilestoreFile, err := createOverlayFilestore(conf) + if err != nil { + return err + } // Join cgroup to start gofer process to ensure it's part of the cgroup from // the start (and all their children processes). if err := runInCgroup(c.Sandbox.CgroupJSON.Cgroup, func() error { @@ -428,7 +435,7 @@ func (c *Container) Start(conf *config.Config) error { stdios = []*os.File{os.Stdin, os.Stdout, os.Stderr} } - return c.Sandbox.StartSubcontainer(c.Spec, conf, c.ID, stdios, goferFiles) + return c.Sandbox.StartSubcontainer(c.Spec, conf, c.ID, stdios, goferFiles, overlayFilestoreFile) }); err != nil { return err } @@ -775,7 +782,8 @@ func (c *Container) Destroy() error { return fmt.Errorf(strings.Join(errs, "\n")) } -func createOverlayFilestore(overlay2 config.Overlay2) (*os.File, error) { +func createOverlayFilestore(conf *config.Config) (*os.File, error) { + overlay2 := conf.GetOverlay2() if !overlay2.IsBackedByHostFile() { return nil, nil } @@ -799,6 +807,9 @@ func createOverlayFilestore(overlay2 config.Overlay2) (*os.File, error) { if err := unix.Unlink(filestoreFile.Name()); err != nil { return nil, fmt.Errorf("failed to unlink temporary file %q: %v", filestoreFile.Name(), err) } + // Perform this work around outside the sandbox. The sandbox may already be + // running with seccomp filters that do not allow this. + pgalloc.IMAWorkAroundForMemFile(filestoreFile.Fd()) return filestoreFile, nil } diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index 23f799953..07f81bfad 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -2214,84 +2214,88 @@ func TestMultiContainerShm(t *testing.T) { } } +// TODO(b/241832602): Revive this test when rootfs medium is added. +// Sandbox.OverlayFileUsage() is no longer supported so stat the named rootfs +// file and use stat.st_blocks to see usage. + // Test that using file-backed overlay does not lead to memory leak or leaks // in the host-file backing the overlay. -func TestMultiContainerOverlayLeaks(t *testing.T) { - conf := testutil.TestConfig(t) - app, err := testutil.FindFile("test/cmd/test_app/test_app") - if err != nil { - t.Fatal("error finding test_app:", err) - } +// func TestMultiContainerOverlayLeaks(t *testing.T) { +// conf := testutil.TestConfig(t) +// app, err := testutil.FindFile("test/cmd/test_app/test_app") +// if err != nil { +// t.Fatal("error finding test_app:", err) +// } - rootDir, cleanup, err := testutil.SetupRootDir() - if err != nil { - t.Fatalf("error creating root dir: %v", err) - } - defer cleanup() - conf.RootDir = rootDir +// rootDir, cleanup, err := testutil.SetupRootDir() +// if err != nil { +// t.Fatalf("error creating root dir: %v", err) +// } +// defer cleanup() +// conf.RootDir = rootDir - // Configure root overlay backed by a file from /tmp. - conf.Overlay2 = config.Overlay2{ - RootMount: true, - Medium: "dir=/tmp", - } +// // Configure root overlay backed by a file from /tmp. +// conf.Overlay2 = config.Overlay2{ +// RootMount: true, +// Medium: "dir=/tmp", +// } - // Root container will just sleep. - sleep := []string{"sleep", "100"} - // Since all containers share the same conf.RootDir, and root filesystems - // have overlay enabled, the root directory should never be modified. Hence, - // creating files at the same locations should not lead to EEXIST error. - createFsTree := []string{app, "fsTreeCreate", "--depth=10", "--file-per-level=10", "--file-size=4096"} - testSpecs, ids := createSpecs(sleep, createFsTree, createFsTree, createFsTree) - // Make sure none of the root filesystems are read-only, otherwise we won't - // be able to create the file. - for _, s := range testSpecs { - s.Root.Readonly = false - } +// // Root container will just sleep. +// sleep := []string{"sleep", "100"} +// // Since all containers share the same conf.RootDir, and root filesystems +// // have overlay enabled, the root directory should never be modified. Hence, +// // creating files at the same locations should not lead to EEXIST error. +// createFsTree := []string{app, "fsTreeCreate", "--depth=10", "--file-per-level=10", "--file-size=4096"} +// testSpecs, ids := createSpecs(sleep, createFsTree, createFsTree, createFsTree) +// // Make sure none of the root filesystems are read-only, otherwise we won't +// // be able to create the file. +// for _, s := range testSpecs { +// s.Root.Readonly = false +// } - // Start the root container. - rootCont, cleanup, err := startContainers(conf, testSpecs[:1], ids[:1]) - if err != nil { - t.Fatalf("error starting containers: %v", err) - } - defer cleanup() +// // Start the root container. +// rootCont, cleanup, err := startContainers(conf, testSpecs[:1], ids[:1]) +// if err != nil { +// t.Fatalf("error starting containers: %v", err) +// } +// defer cleanup() - // Remember the overlay filestore usage right now. - oldOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() - if err != nil { - t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) - } +// // Remember the overlay filestore usage right now. +// oldOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() +// if err != nil { +// t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) +// } - subConts, cleanup, err := startContainers(conf, testSpecs[1:], ids[1:]) - if err != nil { - t.Fatalf("error starting containers: %v", err) - } - defer cleanup() +// subConts, cleanup, err := startContainers(conf, testSpecs[1:], ids[1:]) +// if err != nil { +// t.Fatalf("error starting containers: %v", err) +// } +// defer cleanup() - for i, c := range subConts { - // Wait for the sub-container to stop. - if ws, err := c.Wait(); err != nil { - t.Errorf("failed to wait for subcontainer number %d: %v", i, err) - } else if es := ws.ExitStatus(); es != 0 { - t.Errorf("subcontainer number %d exited with non-zero status %d", i, es) - } - } +// for i, c := range subConts { +// // Wait for the sub-container to stop. +// if ws, err := c.Wait(); err != nil { +// t.Errorf("failed to wait for subcontainer number %d: %v", i, err) +// } else if es := ws.ExitStatus(); es != 0 { +// t.Errorf("subcontainer number %d exited with non-zero status %d", i, es) +// } +// } - // Give the reclaimer goroutine some time to reclaim. - time.Sleep(3 * time.Second) +// // Give the reclaimer goroutine some time to reclaim. +// time.Sleep(3 * time.Second) - // Make sure the overlay filestore usage is back to what it was. The - // sub-containers create files in overlay. But it should have been cleaned - // up once the container exited and the reclaimer ran. - newOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() - if err != nil { - t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) - } +// // Make sure the overlay filestore usage is back to what it was. The +// // sub-containers create files in overlay. But it should have been cleaned +// // up once the container exited and the reclaimer ran. +// newOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() +// if err != nil { +// t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) +// } - if oldOverlayUsage != newOverlayUsage { - t.Errorf("overlay filestore usage changed: old = %d, new = %d", oldOverlayUsage, newOverlayUsage) - } -} +// if oldOverlayUsage != newOverlayUsage { +// t.Errorf("overlay filestore usage changed: old = %d, new = %d", oldOverlayUsage, newOverlayUsage) +// } +// } // Test that spawning many subcontainers that do a lot of filesystem operations // does not lead to memory leaks. diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 41aa5156e..b4ce092cd 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -376,17 +376,23 @@ func (s *Sandbox) StartRoot(spec *specs.Spec, conf *config.Config) error { } // StartSubcontainer starts running a sub-container inside the sandbox. -func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles []*os.File) error { +func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles []*os.File, overlayFilestoreFile *os.File) error { log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load()) if err := s.configureStdios(conf, stdios); err != nil { return err } - // The payload must contain stdin/stdout/stderr (which may be empty if using - // TTY) followed by gofer files. + // The payload contains (in this specific order): + // * stdin/stdout/stderr (optional: only present when not using TTY) + // * The subcontainer's overlay filestore file (optional: only present when + // host file backed overlay is configured) + // * Gofer files. payload := urpc.FilePayload{} payload.Files = append(payload.Files, stdios...) + if overlayFilestoreFile != nil { + payload.Files = append(payload.Files, overlayFilestoreFile) + } payload.Files = append(payload.Files, goferFiles...) // Start running the container. @@ -1265,16 +1271,6 @@ func (s *Sandbox) ChangeLogging(args control.LoggingArgs) error { return nil } -// OverlayFileUsage returns the current usage (bytes) of the overlay filestore. -func (s *Sandbox) OverlayFileUsage() (uint64, error) { - log.Debugf("Getting overlay file usage for sandbox %q", s.ID) - var usage uint64 - if err := s.call(boot.CongMgrOverlayFileUsage, nil, &usage); err != nil { - return 0, fmt.Errorf("getting overlay file usage for sandbox %q: %w", s.ID, err) - } - return usage, nil -} - // DestroyContainer destroys the given container. If it is the root container, // then the entire sandbox is destroyed. func (s *Sandbox) DestroyContainer(cid string) error {