cgroupfs: drop registry entries on filesystem release - #14946
Merged
Merged
Conversation
Commit 26ef517 added CgroupRegistry.RemoveCgroup and called it from dir.RmDir so that destroyed cgroup directories do not stay in kernel.CgroupRegistry.cgroups across save/restore. That fix covers rmdir but two release paths still leak: 1. GetFilesystem error window. newCgroupInode at base.go:190 calls r.AddCgroup for the root cgroup before prepareInitialCgroup and r.Register run. On either failure the code calls rootD.DecRef and fs.VFSFilesystem().DecRef. Release then runs but currently skips ReleaseCgroupHierarchy and Unregister because fs.hierarchyID is still InvalidCgroupHierarchyID. The root cgroup id stays in the registry forever. Repeated mount failures accumulate unbounded entries. 2. prepareInitialCgroup creates intermediate cgroup directories via newDirWithOwner, each of which calls AddCgroup. If prepareInitialCgroup fails partway, the same residue applies. Walk fs.root in Release and call RemoveCgroup for each cgroupInode in the subtree. RemoveCgroup is documented as a no-op for ids not in the map (cgroup.go:667-669), so cgroups that RmDir already removed cost nothing and the rmdir path stays the primary owner. Signed-off-by: Ievgen Bondarenko <sactransport2000@gmail.com>
copybara-service
Bot
force-pushed
the
test/cl986971688
branch
5 times, most recently
from
September 25, 2026 21:39
5a6b778 to
2d0ced4
Compare
…roup PiperOrigin-RevId: 988525356
copybara-service
Bot
force-pushed
the
test/cl986971688
branch
from
September 25, 2026 22:26
2d0ced4 to
bc2e4d8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cgroupfs: drop registry entries on filesystem release
Summary
Follow-up to commit
26ef5174081be0b4b1f750a97e75ee6bad5d5a53(cgroupfs: remove cgroup from CgroupRegistry on rmdir). Walk the cgroupfs dentry subtree in(*filesystem).Releaseand callCgroupRegistry.RemoveCgroupfor everycgroupInode.idin the tree.Why
The rmdir fix covers cgroups removed via
rmdir(2). Two release paths still leak entries:FilesystemType.GetFilesystemerror window atcgroupfs.go:376-398.newCgroupInodeatbase.go:190callsr.AddCgroupfor the root cgroup beforeprepareInitialCgroupandr.Registerrun. On either failure the code doesrootD.DecRef(ctx); fs.VFSFilesystem().DecRef(ctx).Releasethen runs but currently skipsReleaseCgroupHierarchyandUnregisterbecausefs.hierarchyIDis stillInvalidCgroupHierarchyID. The root cgroup id stays inCgroupRegistry.cgroupsforever. Repeated mount failures accumulate unbounded entries that get serialized on every checkpoint, same class as the rmdir leak.prepareInitialCgroupatcgroupfs.go:440-447creates intermediate cgroup directories vianewDirWithOwner, each of which callsAddCgroup. If the path walk later fails, the partially created children leak.Approach
Add a helper
removeCgroupsFromRegistrythat walksfs.rootvia the existingdir.forEachChildDirand callsRemoveCgroupfor eachcgroupInode.id.RemoveCgroupis documented as a no-op for ids that are not present (cgroup.go:667-669), so cgroups already removed byRmDircost nothing.RmDirstays the primary owner of the live-cgroup case; this walk catches entries left at release.Test
bazel test //pkg/sentry/fsimpl/cgroupfs/... //test/syscalls:cgroup_testgit diff --check upstream/master...HEAD. Bazel was unavailable in this environment, so the tests were not rerun after the rebase.CLA
Signed via individual Google CLA on
sactransport2000@gmail.com.FUTURE_COPYBARA_INTEGRATE_REVIEW=#13215 from ibondarenko1:cgroupfs-release-removecgroup b8d03ce