libct/cg/sd: set the DeviceAllow property before DevicePolicy

kolyshkin · wenjianhn · cyphar · kolyshkin · commit d84388ae1037 · 2025-02-05T12:16:43.000-08:00
Every unit created by runc need daemon reload since systemd v230. This breaks support for NVIDIA GPUs, see #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>
diff --git 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
@@ -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"
+}
