Skip to content

Fix/cgroup2 bind mount shared cgroupns#5267

Open
xujihui1985 wants to merge 2 commits into
opencontainers:mainfrom
xujihui1985:fix/cgroup2-bind-mount-shared-cgroupns
Open

Fix/cgroup2 bind mount shared cgroupns#5267
xujihui1985 wants to merge 2 commits into
opencontainers:mainfrom
xujihui1985:fix/cgroup2-bind-mount-shared-cgroupns

Conversation

@xujihui1985

Copy link
Copy Markdown
Contributor

When cgroup namespaces is host, mounting a new cgroup2 fs instance for /sys/fs/cgroup can affect the host-visible cgroupfs mount state, including options such as nsdelegate.

Avoid that by preferring a bind mount of the existing cgroup v2 hierarchy when cgroupns is host. Keep the existing cgroup2 mount-first logic for private cgroup namespaces, including the EPERM/EBUSY fallback to a bind mount and the rootless ENOENT masking behavior.

this PR fix issue #5258

@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch 3 times, most recently from 9a08cc5 to 922bd42 Compare May 3, 2026 07:21
@xujihui1985

Copy link
Copy Markdown
Contributor Author

@lifubang Hi fubang, This PR fixes #5258. I’ve checked the failing CI and it doesn’t seem related to these changes. Please review it when you have time.

@lifubang lifubang force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch from 5c685ee to 0b7a122 Compare May 18, 2026 14:36

@lifubang lifubang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

Comment thread tests/integration/mounts.bats Outdated
@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch 2 times, most recently from 0b7a122 to 9653c5c Compare May 30, 2026 12:41
@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch 2 times, most recently from 1360929 to 491bf97 Compare May 31, 2026 00:41
@xujihui1985

Copy link
Copy Markdown
Contributor Author

Hi @kolyshkin would you like to move forward with this PR?

@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch from 491bf97 to c8ed922 Compare June 9, 2026 03:23
Comment thread tests/integration/mounts.bats Outdated
Comment thread tests/integration/mounts.bats Outdated
Comment thread tests/integration/checkpoint.bats

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to submit pending review comments, my bad.

Overall it's almost ready.

Comment thread tests/integration/mounts.bats Outdated
@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch 4 times, most recently from 3786615 to 3d9d5a7 Compare June 12, 2026 05:52
@xujihui1985

Copy link
Copy Markdown
Contributor Author

@lifubang @kolyshkin I have moved the checkpoint-related commit into a new PR: #5318, and fixed the other review suggestions.

Comment thread libcontainer/rootfs_linux.go Outdated
Comment thread tests/integration/mounts.bats Outdated

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few comments

@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch 2 times, most recently from b76157e to 6c3174f Compare June 27, 2026 05:22
When cgroup namespaces is host, mounting a new cgroup2 fs instance for
/sys/fs/cgroup can affect the host-visible cgroupfs mount state, including
options such as nsdelegate.

Avoid that by preferring a bind mount of the existing cgroup v2 hierarchy
when cgroupns is host. Keep the existing cgroup2 mount-first logic
for private cgroup namespaces, including the EPERM/EBUSY fallback to a
bind mount and the rootless ENOENT masking behavior.

Signed-off-by: sean <xujihui1985@gmail.com>
@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch from 6c3174f to 4e14d47 Compare July 3, 2026 03:07
Comment thread tests/integration/mounts.bats
in host cgroupns, mount cgroupfs should not mutate the global superblock
mount options, the test is to check when create container with host
cgroupns, it should not mutate the global superblock options of host
cgroupfs

Signed-off-by: sean <xujihui1985@gmail.com>
@xujihui1985 xujihui1985 force-pushed the fix/cgroup2-bind-mount-shared-cgroupns branch from 4e14d47 to e8edba3 Compare July 3, 2026 06:08
@xujihui1985

Copy link
Copy Markdown
Contributor Author

@lifubang @kolyshkin I think I have fixed all the issue listed above, anything I can improve here?

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit out of scope for this PR, but I think we should test that the host mounts are not changed after tests (or after each test) as, in general, runc should not change host side mounts (with some exceptions).

@xujihui1985

xujihui1985 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

This is a bit out of scope for this PR, but I think we should test that the host mounts are not changed after tests (or after each test) as, in general, runc should not change host side mounts (with some exceptions).

@kolyshkin yes, the second commit is suppose to do this check e8edba3 so do you think we should run this check for each test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants