From d1dadc9c19a1af820918e2b91725773871458211 Mon Sep 17 00:00:00 2001 From: Rahat Mahmood Date: Mon, 31 Jan 2022 12:39:02 -0800 Subject: [PATCH] Remove dentry_cache_limit mount option from mqfs. The dentry cache limit should only be set at filesystem creation. Mqfs filesystems are created during IPC namespace creation and mount(2)s return a reference to the shared filesystem object. Modifying the cache limit once the filesystem is in use can cause the cache to exceed the limit. Since this mount option is rarely (never?) used, use a static cache size and remove the mount option. Reported-by: syzbot+e89efb5faa374468b6bb@syzkaller.appspotmail.com PiperOrigin-RevId: 425436253 --- pkg/sentry/fsimpl/mqfs/mqfs.go | 30 ++---------------------------- pkg/sentry/fsimpl/mqfs/registry.go | 7 ++++++- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/pkg/sentry/fsimpl/mqfs/mqfs.go b/pkg/sentry/fsimpl/mqfs/mqfs.go index 9b3704217..8c8df220b 100644 --- a/pkg/sentry/fsimpl/mqfs/mqfs.go +++ b/pkg/sentry/fsimpl/mqfs/mqfs.go @@ -18,10 +18,8 @@ package mqfs import ( "fmt" - "strconv" "gvisor.dev/gvisor/pkg/context" - "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/ipc" @@ -31,8 +29,7 @@ import ( const ( // Name is the user-visible filesystem name. - Name = "mqueue" - defaultMaxCachedDentries = uint64(1000) + Name = "mqueue" ) // FilesystemType implements vfs.FilesystemType. @@ -67,34 +64,11 @@ func (ft FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.VirtualF return nil, nil, fmt.Errorf("mqfs.FilesystemType.GetFilesystem: ipc namespace doesn't have a POSIX registry") } impl := registry.Impl().(*RegistryImpl) - - maxCachedDentries, err := maxCachedDentries(ctx, vfs.GenericParseMountOptions(opts.Data)) - if err != nil { - return nil, nil, err - } - impl.fs.MaxCachedDentries = maxCachedDentries - impl.fs.VFSFilesystem().IncRef() impl.root.IncRef() return impl.fs.VFSFilesystem(), impl.root.VFSDentry(), nil } -// maxCachedDentries checks mopts for dentry_cache_limit. If a value is -// specified, parse it into uint64 and return it. Otherwise, return the default -// value. An error is returned if a value is found but can't be parsed. -func maxCachedDentries(ctx context.Context, mopts map[string]string) (_ uint64, err error) { - max := defaultMaxCachedDentries - if str, ok := mopts["dentry_cache_limit"]; ok { - delete(mopts, "dentry_cache_limit") - max, err = strconv.ParseUint(str, 10, 64) - if err != nil { - ctx.Warningf("mqfs.FilesystemType.GetFilesystem: invalid dentry cache limit: dentry_cache_limit=%s", str) - return 0, linuxerr.EINVAL - } - } - return max, nil -} - // filesystem implements kernfs.Filesystem. // // +stateify savable @@ -111,7 +85,7 @@ func (fs *filesystem) Release(ctx context.Context) { // MountOptions implements vfs.FilesystemImpl.MountOptions. func (fs *filesystem) MountOptions() string { - return fmt.Sprintf("dentry_cache_limit=%d", fs.MaxCachedDentries) + return "" } // ipcNamespace defines functions we need from kernel.IPCNamespace. We redefine diff --git a/pkg/sentry/fsimpl/mqfs/registry.go b/pkg/sentry/fsimpl/mqfs/registry.go index 69182965c..28c18b2cd 100644 --- a/pkg/sentry/fsimpl/mqfs/registry.go +++ b/pkg/sentry/fsimpl/mqfs/registry.go @@ -25,6 +25,10 @@ import ( "gvisor.dev/gvisor/pkg/sentry/vfs" ) +const ( + maxCachedDentries = 1000 +) + // RegistryImpl implements mq.RegistryImpl. It implements the interface using // the message queue filesystem, and is provided to mq.Registry at // initialization. @@ -58,7 +62,8 @@ func NewRegistryImpl(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds * } fs := &filesystem{ - devMinor: devMinor, + devMinor: devMinor, + Filesystem: kernfs.Filesystem{MaxCachedDentries: maxCachedDentries}, } fs.VFSFilesystem().Init(vfsObj, &FilesystemType{}, fs) vfsfs := fs.VFSFilesystem()