diff --git a/pkg/sentry/fsimpl/cgroupfs/base.go b/pkg/sentry/fsimpl/cgroupfs/base.go index ed0796eb5..9eab8b103 100644 --- a/pkg/sentry/fsimpl/cgroupfs/base.go +++ b/pkg/sentry/fsimpl/cgroupfs/base.go @@ -203,10 +203,16 @@ func (fs *filesystem) newCgroupInode(ctx context.Context, creds *auth.Credential return c } +// HierarchyID implements kernel.CgroupImpl.HierarchyID. func (c *cgroupInode) HierarchyID() uint32 { return c.fs.hierarchyID } +// Name implements kernel.CgroupImpl.Name. +func (c *cgroupInode) Name() string { + return c.fs.hierarchyName +} + // Controllers implements kernel.CgroupImpl.Controllers. func (c *cgroupInode) Controllers() []kernel.CgroupController { return c.fs.kcontrollers diff --git a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go index addff8d66..2c216003d 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go +++ b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go @@ -149,6 +149,12 @@ type filesystem struct { // hierarchyID is immutable after initialization. hierarchyID uint32 + // hierarchyName is the name for a named hierarchy. May be empty if the + // 'name=' mount option was not used when the hierarchy was created. + // + // Immutable after initialization. + hierarchyName string + // controllers and kcontrollers are both the list of controllers attached to // this cgroupfs. Both lists are the same set of controllers, but typecast // to different interfaces for convenience. Both must stay in sync, and are @@ -236,11 +242,39 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt wantControllers = allControllers } - if len(wantControllers) == 0 { - // Specifying no controllers implies all controllers. + var name string + var ok bool + if name, ok = mopts["name"]; ok { + delete(mopts, "name") + } + + var none bool + if _, ok = mopts["none"]; ok { + none = true + delete(mopts, "none") + } + + if !none && len(wantControllers) == 0 { + // Specifying no controllers implies all controllers, unless "none" was + // explicitly requested. wantControllers = allControllers } + // Some combinations of "none", "all", "name=" and explicit controllers are + // not allowed. See Linux, kernel/cgroup.c:parse_cgroupfs_options(). + + // All empty hierarchies must have a name. + if len(wantControllers) == 0 && name == "" { + ctx.Debugf("cgroupfs.FilesystemType.GetFilesystem: empty hierarchy with no name") + return nil, nil, linuxerr.EINVAL + } + + // Can't have "none" and some controllers. + if none && len(wantControllers) != 0 { + ctx.Debugf("cgroupfs.FilesystemType.GetFilesystem: 'none' specified with controllers: %v", wantControllers) + return nil, nil, linuxerr.EINVAL + } + if len(mopts) != 0 { ctx.Debugf("cgroupfs.FilesystemType.GetFilesystem: unknown options: %v", mopts) return nil, nil, linuxerr.EINVAL @@ -259,7 +293,11 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // // Note: we're guaranteed to have at least one requested controller, since // no explicit controller name implies all controllers. - if vfsfs := r.FindHierarchy(wantControllers); vfsfs != nil { + vfsfs, err := r.FindHierarchy(name, wantControllers) + if err != nil { + return nil, nil, err + } + if vfsfs != nil { fs := vfsfs.Impl().(*filesystem) ctx.Debugf("cgroupfs.FilesystemType.GetFilesystem: mounting new view to hierarchy %v", fs.hierarchyID) fs.root.IncRef() @@ -275,7 +313,8 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // hierarchies. We'll find out about such collisions when we try to register // the new hierarchy later. fs := &filesystem{ - devMinor: devMinor, + devMinor: devMinor, + hierarchyName: name, } fs.MaxCachedDentries = maxCachedDentries fs.VFSFilesystem().Init(vfsObj, &fsType, fs) @@ -337,7 +376,7 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // Register controllers. The registry may be modified concurrently, so if we // get an error, we raced with someone else who registered the same // controllers first. - if err := r.Register(fs.kcontrollers, fs); err != nil { + if err := r.Register(name, fs.kcontrollers, fs); err != nil { ctx.Infof("cgroupfs.FilesystemType.GetFilesystem: failed to register new hierarchy with controllers %v: %v", wantControllers, err) rootD.DecRef(ctx) fs.VFSFilesystem().DecRef(ctx) diff --git a/pkg/sentry/kernel/cgroup.go b/pkg/sentry/kernel/cgroup.go index 511f8fbdb..1a35c92d2 100644 --- a/pkg/sentry/kernel/cgroup.go +++ b/pkg/sentry/kernel/cgroup.go @@ -21,6 +21,8 @@ import ( "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/vfs" ) @@ -101,12 +103,6 @@ func (c *Cgroup) Path() string { return c.FSLocalPath() } -// HierarchyID returns the id of the hierarchy that contains this cgroup. -func (c *Cgroup) HierarchyID() uint32 { - // Note: a cgroup is guaranteed to have at least one controller. - return c.Controllers()[0].HierarchyID() -} - // CgroupMigrationContext represents an in-flight cgroup migration for // a single task. type CgroupMigrationContext struct { @@ -137,6 +133,13 @@ type CgroupImpl interface { // Controllers lists the controller associated with this cgroup. Controllers() []CgroupController + // HierarchyID returns the id of the hierarchy that contains this cgroup. + HierarchyID() uint32 + + // Name returns the name for this cgroup, if any. If no name was provided + // when the hierarchy was created, returns "". + Name() string + // Enter moves t into this cgroup. Enter(t *Task) @@ -174,7 +177,8 @@ type CgroupImpl interface { // // +stateify savable type hierarchy struct { - id uint32 + id uint32 + name string // These are a subset of the controllers in CgroupRegistry.controllers, // grouped here by hierarchy for conveninent lookup. controllers map[CgroupControllerType]CgroupController @@ -220,21 +224,29 @@ type CgroupRegistry struct { mu cgroupMutex `state:"nosave"` // controllers is the set of currently known cgroup controllers on the - // system. Protected by mu. + // system. // // +checklocks:mu controllers map[CgroupControllerType]CgroupController - // hierarchies is the active set of cgroup hierarchies. Protected by mu. + // hierarchies is the active set of cgroup hierarchies. This contains all + // hierarchies on the system. // // +checklocks:mu hierarchies map[uint32]hierarchy + + // hierarchiesByName is a map of named hierarchies. Only named hierarchies + // are tracked on this map. + // + // +checklocks:mu + hierarchiesByName map[string]hierarchy } func newCgroupRegistry() *CgroupRegistry { return &CgroupRegistry{ - controllers: make(map[CgroupControllerType]CgroupController), - hierarchies: make(map[uint32]hierarchy), + controllers: make(map[CgroupControllerType]CgroupController), + hierarchies: make(map[uint32]hierarchy), + hierarchiesByName: make(map[string]hierarchy), } } @@ -247,13 +259,36 @@ func (r *CgroupRegistry) nextHierarchyID() (uint32, error) { } // FindHierarchy returns a cgroup filesystem containing exactly the set of -// controllers named in names. If no such FS is found, FindHierarchy return -// nil. FindHierarchy takes a reference on the returned FS, which is transferred -// to the caller. -func (r *CgroupRegistry) FindHierarchy(ctypes []CgroupControllerType) *vfs.Filesystem { +// controllers named in ctypes, and optionally the name specified in name if it +// isn't empty. If no such FS is found, FindHierarchy return nil. FindHierarchy +// takes a reference on the returned FS, which is transferred to the caller. +func (r *CgroupRegistry) FindHierarchy(name string, ctypes []CgroupControllerType) (*vfs.Filesystem, error) { r.mu.Lock() defer r.mu.Unlock() + // If we have a hierarchy name, lookup by name. + if name != "" { + h, ok := r.hierarchiesByName[name] + if !ok { + // Name not found. + return nil, nil + } + + if h.match(ctypes) { + if !h.fs.TryIncRef() { + // May be racing with filesystem destruction, see below. + r.unregisterLocked(h.id) + return nil, nil + } + return h.fs, nil + } + + // Name matched, but controllers didn't. Fail per linux + // kernel/cgroup.c:cgroup_mount(). + log.Debugf("cgroupfs: Registry lookup for name=%s controllers=%v failed; named matched but controllers didn't (have controllers=%v)", name, ctypes, h.controllers) + return nil, linuxerr.EBUSY + } + for _, h := range r.hierarchies { if h.match(ctypes) { if !h.fs.TryIncRef() { @@ -272,25 +307,25 @@ func (r *CgroupRegistry) FindHierarchy(ctypes []CgroupControllerType) *vfs.Files // dying hierarchy now. The eventual unregister by the FS // teardown will become a no-op. r.unregisterLocked(h.id) - return nil + return nil, nil } - return h.fs + return h.fs, nil } } - return nil + return nil, nil } // Register registers the provided set of controllers with the registry as a new // hierarchy. If any controller is already registered, the function returns an // error without modifying the registry. Register sets the hierarchy ID for the // filesystem on success. -func (r *CgroupRegistry) Register(cs []CgroupController, fs cgroupFS) error { +func (r *CgroupRegistry) Register(name string, cs []CgroupController, fs cgroupFS) error { r.mu.Lock() defer r.mu.Unlock() - if len(cs) == 0 { - return fmt.Errorf("can't register hierarchy with no controllers") + if name == "" && len(cs) == 0 { + return fmt.Errorf("can't register hierarchy with both no controllers and no name") } for _, c := range cs { @@ -299,6 +334,10 @@ func (r *CgroupRegistry) Register(cs []CgroupController, fs cgroupFS) error { } } + if _, ok := r.hierarchiesByName[name]; name != "" && ok { + return fmt.Errorf("hierarchy named %q already exists", name) + } + hid, err := r.nextHierarchyID() if err != nil { return err @@ -310,6 +349,7 @@ func (r *CgroupRegistry) Register(cs []CgroupController, fs cgroupFS) error { h := hierarchy{ id: hid, + name: name, controllers: make(map[CgroupControllerType]CgroupController), fs: fs.VFSFilesystem(), } @@ -319,6 +359,9 @@ func (r *CgroupRegistry) Register(cs []CgroupController, fs cgroupFS) error { h.controllers[n] = c } r.hierarchies[hid] = h + if name != "" { + r.hierarchiesByName[name] = h + } return nil } diff --git a/pkg/sentry/kernel/task_cgroup.go b/pkg/sentry/kernel/task_cgroup.go index f5bfed70c..ce62c48ec 100644 --- a/pkg/sentry/kernel/task_cgroup.go +++ b/pkg/sentry/kernel/task_cgroup.go @@ -194,14 +194,22 @@ func (t *Task) GenerateProcTaskCgroup(buf *bytes.Buffer) { for c := range t.cgroups { ctls := c.Controllers() ctlNames := make([]string, 0, len(ctls)) + + // We're guaranteed to have a valid name, a non-empty controller list, + // or both. + + // Explicit hierachy name, if any. + if name := c.Name(); name != "" { + ctlNames = append(ctlNames, fmt.Sprintf("name=%s", name)) + } + + // Controllers attached to this hierarchy, if any. for _, ctl := range ctls { ctlNames = append(ctlNames, string(ctl.Type())) } cgEntries = append(cgEntries, taskCgroupEntry{ - // Note: We're guaranteed to have at least one controller, and all - // controllers are guaranteed to be on the same hierarchy. - hierarchyID: ctls[0].HierarchyID(), + hierarchyID: c.HierarchyID(), controllers: strings.Join(ctlNames, ","), path: c.Path(), }) diff --git a/test/syscalls/linux/cgroup.cc b/test/syscalls/linux/cgroup.cc index fc34bcd11..b591ae4f8 100644 --- a/test/syscalls/linux/cgroup.cc +++ b/test/syscalls/linux/cgroup.cc @@ -585,6 +585,48 @@ TEST(Cgroup, TIDZeroMovesSelf) { EXPECT_FALSE(tasks.contains(syscall(SYS_gettid))); } +TEST(Cgroup, NamedHierarchies) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + Cgroup c1 = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("none,name=h1")); + Cgroup c2 = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("name=h2,cpu")); + + // Check that /proc//cgroup contains an entry for this task. + absl::flat_hash_map entries = + ASSERT_NO_ERRNO_AND_VALUE(ProcPIDCgroupEntries(getpid())); + EXPECT_TRUE(entries.contains("name=h1")); + EXPECT_TRUE(entries.contains("name=h2,cpu")); + EXPECT_NO_ERRNO(c1.ContainsCallingProcess()); + EXPECT_NO_ERRNO(c2.ContainsCallingProcess()); +} + +TEST(Cgroup, NoneExclusiveWithAnyController) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + EXPECT_THAT(m.MountCgroupfs("none,cpu"), PosixErrorIs(EINVAL, _)); +} + +TEST(Cgroup, EmptyHierarchyMustHaveName) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + // This will fail since it is an empty hierarchy with no name. + EXPECT_THAT(m.MountCgroupfs("none"), PosixErrorIs(EINVAL, _)); +} + +TEST(Cgroup, NameMatchButControllersDont) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + Cgroup c1 = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("none,name=h1")); + Cgroup c2 = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("name=h2,memory")); + + EXPECT_THAT(m.MountCgroupfs("name=h1,memory"), PosixErrorIs(EBUSY, _)); + EXPECT_THAT(m.MountCgroupfs("name=h2,cpu"), PosixErrorIs(EBUSY, _)); +} + TEST(MemoryCgroup, MemoryUsageInBytes) { SKIP_IF(!CgroupsAvailable());