From e54e3668b07ca57efc571bab962af6de856396ea Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Fri, 14 Jul 2023 16:39:59 -0700 Subject: [PATCH] Impose default tmpfs size limits correctly. Syzkaller came up with workloads that fallocate(2) 1 TB in /tmp. The host mlock(2) or madvise(2) syscalls on memfd(2) files end up hanging for multiple minutes in such situations causing the watchdog to mark the calling goroutine as stuck. memfd(2) files have not size limits. Linux fails such fallocate(2) attempts in /tmp with ENOSPC. In Linux tmpfs (shmem), when size= mount option is not specified, the default size limit for the mount is set to 50% of physical RAM size. But in gVisor, it is set to MaxInt64. Which is why Linux fails with ENOSPC and gVisor doesn't. In runsc, the physcial RAM size is already exposed to the containerized application via `/proc/meminfo` which uses `usage.*TotalMemoryBytes`. These fields are configured using the `MemTotal:` field from host `/proc/meminfo`. So use that information to set the default size limit correctly. Reported-by: syzbot+4aa3d6d42b063a11c850@syzkaller.appspotmail.com PiperOrigin-RevId: 548252095 --- pkg/sentry/fsimpl/tmpfs/tmpfs.go | 43 +++++++++++++++++++++----------- pkg/sentry/kernel/kernel.go | 10 +++++++- runsc/boot/loader.go | 9 +++++++ runsc/boot/vfs.go | 3 +++ runsc/cmd/boot.go | 5 ++++ runsc/sandbox/sandbox.go | 4 ++- 6 files changed, 58 insertions(+), 16 deletions(-) diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index d79f52605..6db7a0796 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -140,6 +140,32 @@ type FilesystemOpts struct { // 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 + + // DisableDefaultSizeLimit disables setting a default size limit. In Linux, + // SB_KERNMOUNT has this effect on tmpfs mounts; see mm/shmem.c:shmem_fill_super(). + DisableDefaultSizeLimit bool +} + +// Default size limit mount option. It is immutable after initialization. +var defaultSizeLimit uint64 + +// SetDefaultSizeLimit configures the size limit to be used for tmpfs mounts +// that do not specify a size= mount option. This must be called only once, +// before any tmpfs filesystems are created. +func SetDefaultSizeLimit(sizeLimit uint64) { + defaultSizeLimit = sizeLimit +} + +func getDefaultSizeLimit(disable bool) uint64 { + if disable || defaultSizeLimit == 0 { + // The size limit is used to populate statfs(2) results. If Linux tmpfs is + // mounted with no size option, then statfs(2) returns f_blocks == f_bfree + // == f_bavail == 0. However, many applications treat this as having a size + // limit of 0. To work around this, return a very large but non-zero size + // limit, chosen to ensure that it does not overflow int64. + return math.MaxInt64 + } + return defaultSizeLimit } // GetFilesystem implements vfs.FilesystemType.GetFilesystem. @@ -152,6 +178,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt privateMF := false rootFileType := uint16(linux.S_IFDIR) + disableDefaultSizeLimit := false newFSType := vfs.FilesystemType(&fstype) tmpfsOpts, tmpfsOptsOk := opts.InternalData.(FilesystemOpts) if tmpfsOptsOk { @@ -161,6 +188,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt if tmpfsOpts.FilesystemType != nil { newFSType = tmpfsOpts.FilesystemType } + disableDefaultSizeLimit = tmpfsOpts.DisableDefaultSizeLimit if tmpfsOpts.FilestoreFD != nil { mfOpts := pgalloc.MemoryFileOpts{ // tmpfsOpts.FilestoreFD may be backed by a file on disk (not memfd), @@ -231,15 +259,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt } rootKGID = kgid } - // In Linux, the default size limit is set to 50% of physical RAM. Such a - // default is not suitable in gVisor, because we don't want to reveal the - // host's RAM size. If Linux tmpfs is mounted with no size option, then - // statfs(2) returns f_blocks == f_bfree == f_bavail == 0. - // However, many applications treat this as having a size limit - // of 0. To work around these, set a very large but non-zero default size - // limit, chosen to ensure that BlockSize * Blocks does not overflow int64 - // (which applications may also handle incorrectly). - maxSizeInPages := uint64(math.MaxInt64 / hostarch.PageSize) + maxSizeInPages := getDefaultSizeLimit(disableDefaultSizeLimit) / hostarch.PageSize maxSizeStr, ok := mopts["size"] if ok { delete(mopts, "size") @@ -303,11 +323,6 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return &fs.vfsfs, &root.vfsd, nil } -// NewFilesystem returns a new tmpfs filesystem. -func NewFilesystem(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth.Credentials) (*vfs.Filesystem, *vfs.Dentry, error) { - return FilesystemType{}.GetFilesystem(ctx, vfsObj, creds, "", vfs.GetFilesystemOptions{}) -} - // Release implements vfs.FilesystemImpl.Release. func (fs *filesystem) Release(ctx context.Context) { fs.vfsfs.VirtualFilesystem().PutAnonBlockDevMinor(fs.devMinor) diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index d9b1eb4c8..e97793692 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -439,7 +439,15 @@ func (k *Kernel) Init(args InitKernelArgs) error { pipeMount := k.vfs.NewDisconnectedMount(pipeFilesystem, nil, &vfs.MountOptions{}) k.pipeMount = pipeMount - tmpfsFilesystem, tmpfsRoot, err := tmpfs.NewFilesystem(ctx, &k.vfs, auth.NewRootCredentials(k.rootUserNamespace)) + tmpfsOpts := vfs.GetFilesystemOptions{ + InternalData: tmpfs.FilesystemOpts{ + // See mm/shmem.c:shmem_init() => vfs_kern_mount(flags=SB_KERNMOUNT). + // Note how mm/shmem.c:shmem_fill_super() does not provide a default + // value for sbinfo->max_blocks when SB_KERNMOUNT is set. + DisableDefaultSizeLimit: true, + }, + } + tmpfsFilesystem, tmpfsRoot, err := tmpfs.FilesystemType{}.GetFilesystem(ctx, &k.vfs, auth.NewRootCredentials(k.rootUserNamespace), "", tmpfsOpts) if err != nil { return fmt.Errorf("failed to create tmpfs filesystem: %v", err) } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index e96176145..8a0063153 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -40,6 +40,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/control" "gvisor.dev/gvisor/pkg/sentry/fdimport" "gvisor.dev/gvisor/pkg/sentry/fsimpl/host" + "gvisor.dev/gvisor/pkg/sentry/fsimpl/tmpfs" "gvisor.dev/gvisor/pkg/sentry/fsimpl/user" "gvisor.dev/gvisor/pkg/sentry/inet" "gvisor.dev/gvisor/pkg/sentry/kernel" @@ -260,6 +261,8 @@ type Args struct { // TotalMem is the initial amount of total memory to report back to the // container. TotalMem uint64 + // TotalHostMem is the total memory reported by host /proc/meminfo. + TotalHostMem uint64 // UserLogFD is the file descriptor to write user logs to. UserLogFD int // ProductName is the value to show in @@ -412,6 +415,12 @@ func New(args Args) (*Loader, error) { log.Infof("CPUs: %d", args.NumCPU) runtime.GOMAXPROCS(args.NumCPU) + if args.TotalHostMem > 0 { + // As per tmpfs(5), the default size limit is 50% of total physical RAM. + // See mm/shmem.c:shmem_default_max_blocks(). + tmpfs.SetDefaultSizeLimit(args.TotalHostMem / 2) + } + if args.TotalMem > 0 { // Adjust the total memory returned by the Sentry so that applications that // use /proc/meminfo can make allocations based on this limit. diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index ff3dc9876..ca325c39f 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -550,6 +550,9 @@ func (c *containerMounter) configureOverlay(ctx context.Context, conf *config.Co tmpfsOpts := tmpfs.FilesystemOpts{ RootFileType: uint16(rootType), FilestoreFD: filestoreFD, + // If a mount is being overlaid, it should not be limited by the default + // tmpfs size limit. + DisableDefaultSizeLimit: true, } upperOpts.GetFilesystemOptions.InternalData = tmpfsOpts upper, err := c.k.VFS().MountDisconnected(ctx, creds, "" /* source */, tmpfs.Name, &upperOpts) diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index 2e10c1d5c..10717c90d 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -113,6 +113,9 @@ type Boot struct { // container. totalMem uint64 + // totalHostMem is the total memory reported by host /proc/meminfo. + totalHostMem uint64 + // userLogFD is the file descriptor to write user logs to. userLogFD int @@ -178,6 +181,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.IntVar(&b.procMountSyncFD, "proc-mount-sync-fd", -1, "file descriptor that has to be written to when /proc isn't needed anymore and can be unmounted") f.IntVar(&b.syncUsernsFD, "sync-userns-fd", -1, "file descriptor used to synchronize rootless user namespace initialization.") f.Uint64Var(&b.totalMem, "total-memory", 0, "sets the initial amount of total memory to report back to the container") + f.Uint64Var(&b.totalHostMem, "total-host-memory", 0, "total memory reported by host /proc/meminfo") f.BoolVar(&b.attached, "attached", false, "if attached is true, kills the sandbox process when the parent process terminates") f.StringVar(&b.productName, "product-name", "", "value to show in /sys/devices/virtual/dmi/id/product_name") @@ -406,6 +410,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma OverlayMediums: b.overlayMediums.GetArray(), NumCPU: b.cpuNum, TotalMem: b.totalMem, + TotalHostMem: b.totalHostMem, UserLogFD: b.userLogFD, ProductName: b.productName, PodInitConfigFD: b.podInitConfigFD, diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 1e9d549ff..6d6d8ca67 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -978,11 +978,13 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn // because it relies on stdin being the next FD donated. donations.Donate("stdio-fds", stdios[:]...) - mem, err := totalSystemMemory() + totalSysMem, err := totalSystemMemory() if err != nil { return err } + cmd.Args = append(cmd.Args, "--total-host-memory", strconv.FormatUint(totalSysMem, 10)) + mem := totalSysMem if s.CgroupJSON.Cgroup != nil { cpuNum, err := s.CgroupJSON.Cgroup.NumCPU() if err != nil {