fix(container): make --device-cgroup-rule allow list self-contained by TDnorthgarden · Pull Request #5319 · containerd/nerdctl

TDnorthgarden · 2026-11-02T08:56:40Z

Problem

--device-cgroup-rule (added in #5258) appends the user's rules to the
container's device cgroup allow list. The spec already carries the standard
device grants behind a leading deny-all rule, because nerdctl applies
oci.WithDefaultSpec and then oci.WithDefaultUnixDevices -- so /dev/null and
friends are covered. What the spec does not list is the rest of the list that
runc and crun add implicitly (specconv.AllowedDevices): the mknod wildcards and
/dev/net/tun. That implicit part is a runtime implementation detail, not part of
the runtime-spec, so a runtime that applies the spec's device list verbatim
would deny those devices.

The comment on withDeviceCgroupRules also claimed the list "starts with an
entry denying every device" and implied the standard devices depend on the
runtime adding them; neither matches what nerdctl actually builds.

Fix

Append the runc/crun default entries and then the user rules behind the
rules already in the spec, so the leading deny-all rule stays first and no
deny-all ends up behind a grant (a device cgroup applies its rules in order).
Leave the wildcard major/minor fields unset instead of -1: runc reads a
missing field as -1, while crun writes a present -1 verbatim ("c -1:-1 m"),
which the kernel rejects on cgroup v1.
Fix the comment on withDeviceCgroupRules and make the unit test use the
real base spec (deny-all plus the WithDefaultUnixDevices grants) with full
order assertions.

Unit tests cover: (1) defaults and user rules appended after the real base
rules, (2) coexistence with --device grants, (3) no-op when the flag is not
used, (4) no explicit -1 wildcard in the defaults.

AkihiroSuda · 2026-11-02T10:16:04Z

Can we have an integration test too?

AkihiroSuda · 2026-11-02T10:52:37Z


+func TestRunDeviceCgroupRuleDefaultDevices(t *testing.T) {
+	testCase := nerdtest.Setup()
+	testCase.Require = require.Not(nerdtest.Docker)


Why not compatible with Docker?

You are right, the guard was wrong: the docker CLI has had the same
--device-cgroup-rule flag since moby/moby#22563, so these tests can run
in docker mode. I have removed it and verified every subtest against both
CLIs (docker 29.9.2; nerdctl on containerd 2.4.2 + runc 1.5.2):

"rules are accepted" and "standard devices remain accessible" pass
as-is under docker — a run with only "c 189:* rmw" still has /dev/null,
/dev/zero and /dev/urandom accessible, same as nerdctl;

the malformed-rule subtests also pass under docker, now that the expected
error matches the wording shared by both CLIs (nerdctl: "invalid device
cgroup rule format", docker: "invalid device cgroup format").

Mind taking another look when you get a chance?

AkihiroSuda · 2026-11-04T05:00:40Z

+// friends on a runtime that applies the spec's list verbatim. When the flag is
+// used, include the defaults explicitly so the allow list is self-contained.
+//
+// Keep in sync with runc's specconv.AllowedDevices (as of runc v1.1.15).


Why refer to a very old version of runc?

You're right that the reference was stale. I diffed our copy against runc's specconv.AllowedDevices in v1.5.2 (current stable; also unchanged on main): all 11 rules still match rule-for-rule, so only the comment was out of date. Updated it to (unchanged from v1.1.15 through v1.5.2, the current stable release) in 57530b3.

AkihiroSuda · 2026-11-04T05:00:51Z

Please squash the commits

AkihiroSuda

Thanks

Copilot

🟡 Changes recommended

Incorrect device-rule ordering cancels user grants, while explicit -1 wildcard fields produce kernel-rejected rules that prevent non-privileged containers from starting with crun on cgroup v1.

4 open issues

What changed in this PR

This PR aims to make nerdctl’s --device-cgroup-rule allow lists independent of runtime-provided defaults while preserving access to standard container devices.

Changes:

Adds explicit default device allowances when custom rules are supplied.
Updates unit tests and adds an integration check for standard-device access.
Matches the common malformed-rule error prefix across nerdctl and Docker.

File	Description
`pkg/cmd/container/run_cgroup_linux.go`	Adds default device rules and changes how existing and custom rules are combined.
`pkg/cmd/container/run_cgroup_linux_test.go`	Revises fixtures and assertions for defaults, existing device grants, and no-rule behavior.
`cmd/nerdctl/container/container_run_cgroup_linux_test.go`	Adds standard-device access coverage and adjusts error matching.

_{Review effort: Max}

Copilot

🟡 Changes recommended

The test copies its expected defaults from defaultDeviceCgroupRules, so changes or removal of the /dev/net/tun grant would go undetected without an independent assertion.

1 open issue

Add an independent assertion for the /dev/net/tun grant · New

4 resolved since last review

_{Review effort: Max}

Copilot

🟡 Changes recommended

Fallback device grants can widen explicitly read-only device mappings to allow writes and cause container creation to fail under restrictive cgroup v1 parents.

3 open issues

1 resolved since last review

Add an independent assertion for the /dev/net/tun grant

_{Review effort: Max}

+		}
+		devices := make([]specs.LinuxDeviceCgroup, 0, len(s.Linux.Resources.Devices)+len(defaultDeviceCgroupRules)+len(rules))
+		devices = append(devices, s.Linux.Resources.Devices...)
+		devices = append(devices, defaultDeviceCgroupRules...)


+			Description: "standard devices remain accessible",
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				return helpers.Command("run", "--rm",
+					"--device-cgroup-rule", "c 189:* rmw",


+	{Allow: true, Type: "c", Access: "m"},
+	{Allow: true, Type: "b", Access: "m"},


The device cgroup allow list built by --device-cgroup-rule only has the user's rules on top of what the spec already carries. The spec does hold the standard device grants (oci.WithDefaultUnixDevices) behind the leading deny-all rule from oci.WithDefaultSpec, but it lists neither the mknod wildcards nor /dev/net/tun, and those are only added implicitly by runc (specconv.AllowedDevices) and crun. A runtime that applies the spec's device list verbatim would deny them. When --device-cgroup-rule is used, append the runc/crun default entries and then the user rules behind the rules already in the spec, so that: - the leading deny-all rule stays first and no deny-all ends up behind a grant, which would cancel it (a device cgroup applies its rules in order); - the allow list no longer depends on the runtime adding its own defaults. A default entry is skipped when the spec already grants that device, which is what oci.WithDevices does for --device: mapping /dev/net/tun as read-only must not be widened to "rwm" by the fallback grants. The standard devices are already listed by oci.WithDefaultUnixDevices, so only the gaps are filled in. ParseDeviceCgroupRule used to spell a "*" major or minor as -1 and always set both fields. runc reads a missing field as -1 itself, so that worked there, but crun writes whatever is present verbatim: a rule such as --device-cgroup-rule "c 189:* rmw" reached the kernel as "c 189:-1 rmw" on cgroup v1, which the kernel rejects, so the container failed to start. Leave the wildcard unset instead, which both runtimes read as "any"; the defaults follow the same convention. Also fix the comment on withDeviceCgroupRules, which claimed the list "starts with an entry denying every device" and that the standard devices depend on runtime-added rules; no such fabricated entry exists, and the spec already lists most of those devices. Tests: - unit: TestWithDeviceCgroupRules now uses the real base spec (the deny-all rule plus the WithDefaultUnixDevices grants) and asserts the complete rule order, including that user rules come after the deny-all rule. The /dev/net/tun grant (c 10:200 rwm) is pinned as a literal in the resulting spec instead of being read back from defaultDeviceCgroupRules, a read-only --device mapping of the same device keeps its "r" instead of being widened, the entries the spec already covers are not appended twice, and parsed wildcards stay unset. TestDefaultDeviceCgroupRulesWildcards asserts the same on the defaults list and guards against explicit -1 wildcards. - TestParseDeviceCgroupRule expects unset major/minor for "*". - integration: TestRunDeviceCgroupRuleDefaultDevices starts a container with only "c 189:* rmw" and asserts /dev/null, /dev/zero and /dev/urandom stay accessible. The docker CLI has the same flag (moby/moby#22563), so the test also runs in docker mode, and the malformed-rule rejection matches the wording shared by both CLIs. Signed-off-by: Aucoto Zhang <tdnorthgarden@gmail.com>

AkihiroSuda added this to the v2.5.0 (tentative) milestone Nov 2, 2026

AkihiroSuda added the cherry-pick/2.4 label Nov 2, 2026

AkihiroSuda reviewed Nov 2, 2026

View reviewed changes

Comment thread pkg/cmd/container/run_cgroup_linux.go

AkihiroSuda Nov 2, 2026

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.

Can we have an integration test too?

AkihiroSuda reviewed Nov 2, 2026

View reviewed changes

TDnorthgarden requested a review from AkihiroSuda November 4, 2026 03:53

AkihiroSuda reviewed Nov 4, 2026

View reviewed changes

TDnorthgarden force-pushed the fix/device-cgroup-rule-explicit-defaults branch from 03c84d5 to 57530b3 Compare November 4, 2026 05:51

AkihiroSuda approved these changes Nov 4, 2026

View reviewed changes

AkihiroSuda requested a max review from Copilot November 4, 2026 06:00

Copilot started reviewing on behalf of AkihiroSuda November 4, 2026 06:00

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread pkg/cmd/container/run_cgroup_linux.go Resolved

Comment thread pkg/cmd/container/run_cgroup_linux.go Resolved

Comment thread pkg/cmd/container/run_cgroup_linux.go Resolved

Comment thread pkg/cmd/container/run_cgroup_linux_test.go Resolved

TDnorthgarden force-pushed the fix/device-cgroup-rule-explicit-defaults branch from 57530b3 to ba6ccc9 Compare November 4, 2026 07:59

TDnorthgarden requested review from AkihiroSuda and a max review from Copilot November 5, 2026 02:50

Copilot started reviewing on behalf of TDnorthgarden November 5, 2026 02:50

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread pkg/cmd/container/run_cgroup_linux_test.go Resolved

TDnorthgarden force-pushed the fix/device-cgroup-rule-explicit-defaults branch from ba6ccc9 to c925fa4 Compare November 11, 2026 01:48

TDnorthgarden requested a max review from Copilot November 11, 2026 02:41

Copilot started reviewing on behalf of TDnorthgarden November 11, 2026 02:41

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

TDnorthgarden force-pushed the fix/device-cgroup-rule-explicit-defaults branch from c925fa4 to db463d2 Compare November 11, 2026 10:51

		{Allow: true, Type: "c", Access: "m"},
		{Allow: true, Type: "b", Access: "m"},

Conversation

TDnorthgarden commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Uh oh!

AkihiroSuda Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

TDnorthgarden Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

TDnorthgarden Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

TDnorthgarden Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TDnorthgarden commented Nov 2, 2026 •

edited

Loading