From d84388ae10370654f9320addf4c9e364862165d8 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 4 Feb 2025 16:51:43 -0800 Subject: [PATCH] libct/cg/sd: set the DeviceAllow property before DevicePolicy Every unit created by runc need daemon reload since systemd v230. This breaks support for NVIDIA GPUs, see https://github.com/opencontainers/runc/issues/3708#issuecomment-2216967210 A workaround is to set DeviceAllow before DevicePolicy. Also: - add a test case (which fails before the fix) by @kolyshkin - better explain why we need empty DeviceAllow (by @cyphar) Fixes 4568. Reported-by: Jian Wen Co-authored-by: Jian Wen Co-authored-by: Aleksa Sarai Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/devices/systemd.go | 14 +++++++++++--- tests/integration/dev.bats | 12 ++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/libcontainer/cgroups/devices/systemd.go b/libcontainer/cgroups/devices/systemd.go index 4851b56137d..9627efd5262 100644 --- a/libcontainer/cgroups/devices/systemd.go +++ b/libcontainer/cgroups/devices/systemd.go @@ -23,10 +23,18 @@ func systemdProperties(r *cgroups.Resources, sdVer int) ([]systemdDbus.Property, } properties := []systemdDbus.Property{ + // When we later add DeviceAllow=/dev/foo properties, we are + // appending devices to the allow list for the unit. However, + // if this is an existing unit, it already has DeviceAllow= + // entries, and we need to clear them all before applying the + // new set. (We also do this for new units, mainly for safety + // to ensure we only enable the devices we expect.) + // + // To clear any existing DeviceAllow= rules, we have to add an + // empty DeviceAllow= property. + newProp("DeviceAllow", []deviceAllowEntry{}), // Always run in the strictest white-list mode. newProp("DevicePolicy", "strict"), - // Empty the DeviceAllow array before filling it. - newProp("DeviceAllow", []deviceAllowEntry{}), } // Figure out the set of rules. @@ -239,7 +247,7 @@ func allowAllDevices() []systemdDbus.Property { // Setting mode to auto and removing all DeviceAllow rules // results in allowing access to all devices. return []systemdDbus.Property{ - newProp("DevicePolicy", "auto"), newProp("DeviceAllow", []deviceAllowEntry{}), + newProp("DevicePolicy", "auto"), } } diff --git a/tests/integration/dev.bats b/tests/integration/dev.bats index 9da472d1acc..6493c1c0388 100644 --- a/tests/integration/dev.bats +++ b/tests/integration/dev.bats @@ -141,3 +141,15 @@ function teardown() { runc exec -t test_exec sh -c "ls -l /proc/self/fd/0; echo 123" [ "$status" -eq 0 ] } + +# https://github.com/opencontainers/runc/issues/4568 +@test "runc run [devices vs systemd NeedDaemonReload]" { + # The systemd bug is there since v230, see + # https://github.com/systemd/systemd/pull/3170/commits/ab932a622d57fd327ef95992c343fd4425324088 + # and https://github.com/systemd/systemd/issues/35710. + requires systemd_v230 + + set_cgroups_path + runc run -d --console-socket "$CONSOLE_SOCKET" test_need_reload + check_systemd_value "NeedDaemonReload" "no" +}