Skip to content

Commit

Permalink
libct/cg/sd: set the DeviceAllow property before DevicePolicy
Browse files Browse the repository at this point in the history
Every unit created by runc need daemon reload since systemd v230.
This breaks support for NVIDIA GPUs, see
opencontainers#3708 (comment)

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 <wenjianhn@gmail.com>
Co-authored-by: Jian Wen <wenjianhn@gmail.com>
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
3 people committed Feb 5, 2025
1 parent a5bfdc9 commit d84388a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
14 changes: 11 additions & 3 deletions libcontainer/cgroups/devices/systemd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"),
}
}
12 changes: 12 additions & 0 deletions tests/integration/dev.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

0 comments on commit d84388a

Please sign in to comment.