From 8487a06cd9172bdb4033c11a6337a45bb983721d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 10 Apr 2026 18:21:36 -0700 Subject: [PATCH 1/3] libct/specconv: fixup TestCreateDevices Commit 0709202d added TestCreateDevices which, among the other things, checks that defaultDevs, as returned by createDevices, does not contain a *second* entry for /dev/tty in case user-supplied rules also have an entry to /dev/tty. Since defaultDevs is a subset of AllowedDevices, having two entries is not possible at all, making this check useless. What it should probably check is that defaultDevs does not contain an entry for /dev/tty (as the user-supplied one prevails). Fix the test appropriately. While at it, do some minor cleanup. The alternative to this is to remove this check entirely. Fixes: 0709202d Signed-off-by: Kir Kolyshkin --- libcontainer/specconv/spec_linux_test.go | 36 +++++++++++------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 3e532d490fe..184d71cf94b 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -865,21 +865,22 @@ func TestNullProcess(t *testing.T) { func TestCreateDevices(t *testing.T) { spec := Example() - // dummy uid/gid for /dev/tty; will enable the test to check if createDevices() - // preferred the spec's device over the redundant default device + // Dummy uid/gid for /dev/tty; will enable the test to check if createDevices() + // preferred the spec's device over the redundant default device. ttyUid := uint32(1000) ttyGid := uint32(1000) - fm := os.FileMode(0o666) + ttyMode := os.FileMode(0o666) + ttyPath := "/dev/tty" spec.Linux = &specs.Linux{ Devices: []specs.LinuxDevice{ { // This is purposely redundant with one of runc's default devices - Path: "/dev/tty", + Path: ttyPath, Type: "c", Major: 5, Minor: 0, - FileMode: &fm, + FileMode: &ttyMode, UID: &ttyUid, GID: &ttyGid, }, @@ -900,14 +901,11 @@ func TestCreateDevices(t *testing.T) { t.Errorf("failed to create devices: %v", err) } - // Verify the returned default devices has the /dev/tty entry deduplicated - found := false - for _, d := range defaultDevs { - if d.Path == "/dev/tty" { - if found { - t.Errorf("createDevices failed: returned a duplicated device entry: %v", defaultDevs) - } - found = true + // Verify that /dev/tty is not in defaultDevs, + // because the one in spec is preferred. + for _, dev := range defaultDevs { + if dev.Path == ttyPath { + t.Errorf("%s should not be present in defaultDevs", ttyPath) } } @@ -934,12 +932,12 @@ func TestCreateDevices(t *testing.T) { // Verify that createDevices() deduplicated the /dev/tty entry in the config for _, configDev := range conf.Devices { - if configDev.Path == "/dev/tty" { + if configDev.Path == ttyPath { wantDev := &devices.Device{ - Path: "/dev/tty", - FileMode: 0o666, - Uid: 1000, - Gid: 1000, + Path: ttyPath, + FileMode: ttyMode, + Uid: ttyUid, + Gid: ttyGid, Rule: devices.Rule{ Type: devices.CharDevice, Major: 5, @@ -954,7 +952,7 @@ func TestCreateDevices(t *testing.T) { } // Verify that createDevices() added the entry for /dev/ram0 in the config - found = false + found := false for _, configDev := range conf.Devices { if configDev.Path == "/dev/ram0" { found = true From 7a2942fa4cccf54bc25d801883c761cc9a6951f2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 13 Apr 2026 10:18:22 -0700 Subject: [PATCH 2/3] tests/int: demo default device access rule removal Since commit 0709202d ("Remove runc default devices that overlap with spec devices.") runc removes the default cgroup device access rule from the default set in case a device with the same path is also listed in container spec. Judging by the commit description, this was not the intention, and yet this is what we have. As the behavior is now part of runc (since v1.0-rc93), it makes sense to at least test it, to ensure it won't be broken in the future. Note that the above behavior is only for rootful runc (rootless does not use cgroup device access rules and bind-mounts host devices instead). In addition, the test case serves as a demo how to limit the container device access to a subset of default AllowedDevices, which we thought was not possible before. Turns out it is possible, in a way (and for rootful runc only). Signed-off-by: Kir Kolyshkin --- tests/integration/dev.bats | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/integration/dev.bats b/tests/integration/dev.bats index 6493c1c0388..57bc2f0b538 100644 --- a/tests/integration/dev.bats +++ b/tests/integration/dev.bats @@ -10,6 +10,36 @@ function teardown() { teardown_bundle } +@test "runc run [redundant default /dev/full]" { + requires root # Rootless devices behave differently. + + # 1. This is how a device from the default AllowedDevices should work as is. + # It's /dev/full so it should return "no space left on device" error. + update_config ' .process.args |= ["sh", "-c", "stat /dev/full; echo foo >/dev/full"]' + runc run test_dev + [ "$status" -eq 1 ] + [[ "$output" == *"Device type: 1,7"* ]] + [[ "$output" == *": No space left on device"* ]] + + # 2. Add the device to linux.devices only (but not to linux.resources.devices). + # This way it will be excluded from the cgroup allow rules. + update_config ' .linux.devices += [{"path": "/dev/full", "type": "c", "major": 1, "minor": 7}]' + runc run test_dev + [ "$status" -eq 1 ] + [[ "$output" == *"Device type: 1,7"* ]] + [[ "$output" == *": Operation not permitted"* ]] + + # 3. Also add it to cgroups list. Now it should work like the default one (see 1 above). + update_config ' .linux.resources.devices = [ + {"allow": false, "access": "rwm"}, + {"allow": true, "type": "c", "major": 1, "minor": 7, "access": "rw"} + ]' + runc run test_dev + [ "$status" -eq 1 ] + [[ "$output" == *"Device type: 1,7"* ]] + [[ "$output" == *": No space left on device"* ]] +} + @test "runc run [redundant default /dev/tty]" { update_config ' .linux.devices += [{"path": "/dev/tty", "type": "c", "major": 5, "minor": 0}] | .process.args |= ["ls", "-lLn", "/dev/tty"]' From 939f0e5c5b09689a7f6a17cb9c0f4909a865ab2f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 17 Apr 2026 19:23:32 -0700 Subject: [PATCH 3/3] libct/specconv: comment on device.Rule use in createDevices The usage of device.Rule is not very obvious, so explain it. Signed-off-by: Kir Kolyshkin --- libcontainer/specconv/spec_linux.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 5713460e486..2006ba3b7da 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -1060,6 +1060,9 @@ next: filemode = *d.FileMode &^ unix.S_IFMT } device := &devices.Device{ + // NOTE despite the name this is not a device access rule + // (those live under cgroup), but merely a way to specify + // a device type and major:minor for mknod. Rule: devices.Rule{ Type: dt, Major: d.Major,