From 75874136acf7fdff44668bd9401219960a783d1b Mon Sep 17 00:00:00 2001
From: Luke Palmer <luke@lukepalmer.net>
Date: Thu, 13 Jun 2024 09:27:19 -0400
Subject: [PATCH] fix cgroup setup for non-default devices (#22518)

---
 .changelog/22518.txt                          |  3 ++
 drivers/shared/executor/executor_linux.go     |  9 +++---
 .../shared/executor/executor_linux_test.go    | 32 +++++++++++++++++++
 3 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 .changelog/22518.txt

diff --git a/.changelog/22518.txt b/.changelog/22518.txt
new file mode 100644
index 000000000..39a31edbf
--- /dev/null
+++ b/.changelog/22518.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+driver: Fixed a bug where the exec, java, and raw_exec drivers would not configure cgroups to allow access to devices provided by device plugins
+```
diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go
index 7e25237c2..f83d88ec1 100644
--- a/drivers/shared/executor/executor_linux.go
+++ b/drivers/shared/executor/executor_linux.go
@@ -677,6 +677,10 @@ func configureIsolation(cfg *runc.Config, command *ExecCommand) error {
 		cfg.Devices = append(cfg.Devices, devs...)
 	}
 
+	for _, device := range cfg.Devices {
+		cfg.Cgroups.Resources.Devices = append(cfg.Cgroups.Resources.Devices, &device.Rule)
+	}
+
 	cfg.Mounts = []*runc.Mount{
 		{
 			Source:      "tmpfs",
@@ -844,10 +848,6 @@ func (l *LibcontainerExecutor) newLibcontainerConfig(command *ExecCommand) (*run
 		Version: "1.0.0",
 	}
 
-	for _, device := range specconv.AllowedDevices {
-		cfg.Cgroups.Resources.Devices = append(cfg.Cgroups.Resources.Devices, &device.Rule)
-	}
-
 	configureCapabilities(cfg, command)
 
 	// children should not inherit Nomad agent oom_score_adj value
@@ -897,6 +897,7 @@ func cmdDevices(driverDevices []*drivers.DeviceConfig) ([]*devices.Device, error
 			return nil, fmt.Errorf("failed to make device out for %s: %v", d.HostPath, err)
 		}
 		ed.Path = d.TaskPath
+		ed.Allow = true // rules will be used to allow devices via cgroups
 		r[i] = ed
 	}
 
diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go
index d72a55e9f..172bb51e5 100644
--- a/drivers/shared/executor/executor_linux_test.go
+++ b/drivers/shared/executor/executor_linux_test.go
@@ -744,6 +744,7 @@ func TestExecutor_cmdDevices(t *testing.T) {
 			Major:       1,
 			Minor:       3,
 			Permissions: "rwm",
+			Allow:       true,
 		},
 		Path: "/task/dev/null",
 	}
@@ -973,3 +974,34 @@ func TestExecutor_SignalCatching(t *testing.T) {
 	must.NoError(t, err)
 	must.Eq(t, specs.StateStopped, status.Status)
 }
+
+// non-default devices must be present in cgroup device rules
+func TestCgroupDeviceRules(t *testing.T) {
+	ci.Parallel(t)
+	testutil.ExecCompatible(t)
+	testExecCmd := testExecutorCommand(t)
+	command := testExecCmd.command
+
+	allocDir := testExecCmd.allocDir
+	defer allocDir.Destroy()
+
+	command.Devices = append(command.Devices,
+		// /dev/fuse is not in the default device list
+		&drivers.DeviceConfig{
+			HostPath:    "/dev/fuse",
+			TaskPath:    "/dev/fuse",
+			Permissions: "rwm",
+		})
+	execInterface := NewExecutorWithIsolation(testlog.HCLogger(t), compute)
+	executor := execInterface.(*LibcontainerExecutor)
+	cfg, err := executor.newLibcontainerConfig(command)
+	must.NoError(t, err)
+
+	must.SliceContains(t, cfg.Cgroups.Devices, &devices.Rule{
+		Type:        'c',
+		Major:       0x0a,
+		Minor:       0xe5,
+		Permissions: "rwm",
+		Allow:       true,
+	})
+}