interfaces/opengl: don't udev tag nvidia devices and use snap-confine…

… instead

/dev/nvidia* devices are created via the proprietary nvidia module. Proprietary
nvidia modules are not allowed to use sysfs, therefore they cannot be udev
tagged and ultimately won't be added to the device cgroup when the opengl
interface is connected. For now, adjust snap-confine to add /dev/nvidia* to the
device cgroup if they exist, and let AppArmor handle the mediation.

cmd/snap-confine/udev-support.c

@@ -32,27 +32,10 @@
 #include "../libsnap-confine-private/utils.h"
 #include "udev-support.h"
 
-void run_snappy_app_dev_add(struct snappy_udev *udev_s, const char *path)
+void _run_snappy_app_dev_add_majmin(struct snappy_udev *udev_s,
+				    const char *path, unsigned major,
+				    unsigned minor)
 {
-	if (udev_s == NULL)
-		die("snappy_udev is NULL");
-	if (udev_s->udev == NULL)
-		die("snappy_udev->udev is NULL");
-	if (udev_s->tagname_len == 0
-	    || udev_s->tagname_len >= MAX_BUF
-	    || strnlen(udev_s->tagname, MAX_BUF) != udev_s->tagname_len
-	    || udev_s->tagname[udev_s->tagname_len] != '\0')
-		die("snappy_udev->tagname has invalid length");
-
-	debug("%s: %s %s", __func__, path, udev_s->tagname);
-
-	struct udev_device *d =
-	    udev_device_new_from_syspath(udev_s->udev, path);
-	if (d == NULL)
-		die("can not find %s", path);
-	dev_t devnum = udev_device_get_devnum(d);
-	udev_device_unref(d);
-
 	int status = 0;
 	pid_t pid = fork();
 	if (pid < 0) {
@@ -72,8 +55,6 @@ void run_snappy_app_dev_add(struct snappy_udev *udev_s, const char *path)
 		// user-controlled environment can't be used to subvert
 		// snappy-add-dev
 		char *env[] = { NULL };
-		unsigned major = MAJOR(devnum);
-		unsigned minor = MINOR(devnum);
 		sc_must_snprintf(buf, sizeof(buf), "%u:%u", major, minor);
 		execle("/lib/udev/snappy-app-dev", "/lib/udev/snappy-app-dev",
 		       "add", udev_s->tagname, path, buf, NULL, env);
@@ -87,6 +68,32 @@ void run_snappy_app_dev_add(struct snappy_udev *udev_s, const char *path)
 		die("child died with signal %i", WTERMSIG(status));
 }
 
+void run_snappy_app_dev_add(struct snappy_udev *udev_s, const char *path)
+{
+	if (udev_s == NULL)
+		die("snappy_udev is NULL");
+	if (udev_s->udev == NULL)
+		die("snappy_udev->udev is NULL");
+	if (udev_s->tagname_len == 0
+	    || udev_s->tagname_len >= MAX_BUF
+	    || strnlen(udev_s->tagname, MAX_BUF) != udev_s->tagname_len
+	    || udev_s->tagname[udev_s->tagname_len] != '\0')
+		die("snappy_udev->tagname has invalid length");
+
+	debug("%s: %s %s", __func__, path, udev_s->tagname);
+
+	struct udev_device *d =
+	    udev_device_new_from_syspath(udev_s->udev, path);
+	if (d == NULL)
+		die("can not find %s", path);
+	dev_t devnum = udev_device_get_devnum(d);
+	udev_device_unref(d);
+
+	unsigned major = MAJOR(devnum);
+	unsigned minor = MINOR(devnum);
+	_run_snappy_app_dev_add_majmin(udev_s, path, major, minor);
+}
+
 /*
  * snappy_udev_init() - setup the snappy_udev structure. Return 0 if devices
  * are assigned, else return -1. Callers should use snappy_udev_cleanup() to
@@ -197,6 +204,33 @@ void setup_devices_cgroup(const char *security_tag, struct snappy_udev *udev_s)
 	for (int i = 0; static_devices[i] != NULL; i++)
 		run_snappy_app_dev_add(udev_s, static_devices[i]);
 
+	// nvidia modules are proprietary and therefore aren't in sysfs and
+	// can't be udev tagged. For now, just add existing nvidia devices to
+	// the cgroup unconditionally (AppArmor will still mediate the access).
+	// We'll want to rethink this if snapd needs to mediate access to other
+	// proprietary devices.
+	struct stat sbuf;
+	char nvpath[16] = { 0 };	// /dev/nvidiaXXX, ctl and -uvm
+	for (unsigned minor = 0; minor < 256; minor++) {
+		// https://github.com/torvalds/linux/blob/master/Documentation/admin-guide/devices.txt
+		// /dev/nvidia0 through /dev/nvidia254 and /dev/nvidiactl
+		if (minor < 255)
+			sc_must_snprintf(nvpath, sizeof(nvpath),
+					 "/dev/nvidia%u", minor);
+		else
+			sc_must_snprintf(nvpath, sizeof(nvpath),
+					 "/dev/nvidia%s", "ctl");
+
+		if (stat(nvpath, &sbuf) == 0) {
+			_run_snappy_app_dev_add_majmin(udev_s, nvpath, 195,
+						       minor);
+		}
+	}
+
+	sc_must_snprintf(nvpath, sizeof(nvpath), "/dev/nvidia%s", "-uvm");
+	if (stat(nvpath, &sbuf) == 0) {
+		_run_snappy_app_dev_add_majmin(udev_s, nvpath, 247, 0);
+
 	// add the assigned devices
 	while (udev_s->assigned != NULL) {
 		const char *path = udev_list_entry_get_name(udev_s->assigned);

interfaces/builtin/opengl.go

@@ -68,9 +68,10 @@ const openglConnectedPlugAppArmor = `
   /run/udev/data/c226:[0-9]* r,  # 226 drm
 `
 
+// The nvidia modules don't use sysfs (therefore they can't be udev tagged) and
+// will be added by snap-confine.
 const openglConnectedPlugUDev = `
 SUBSYSTEM=="drm", KERNEL=="card[0-9]*", TAG+="###CONNECTED_SECURITY_TAGS###"
-KERNEL=="nvidia*", TAG+="###CONNECTED_SECURITY_TAGS###"
 KERNEL=="vchiq",   TAG+="###CONNECTED_SECURITY_TAGS###"
 `
 

tests/main/security-device-cgroups/task.yaml

@@ -23,8 +23,15 @@ prepare: |
     if [ ! -e /sys/devices/virtual/misc/uinput ]; then
         modprobe uinput
     fi
+    if [ ! -e /dev/nvidia0 ]; then
+        mknod /dev/nvidia0 c 195 0
+        touch /dev/nvidia0.spread
+    fi
 
 restore: |
+    if [ -e /dev/nvidia0.spread ]; then
+        rm -f /dev/nvidia0 /dev/nvidia0.spread
+    fi
     rm -f /etc/udev/rules.d/70-snap.test-snapd-tools.rules
     udevadm control --reload-rules
     udevadm trigger
@@ -70,4 +77,10 @@ execute: |
     echo "And other devices are not shown in the snap device list"
     MATCH -v "$OTHER_DEVICE_ID" < /sys/fs/cgroup/devices/snap.test-snapd-tools.env/devices.list
 
+    echo "But existing nvidia devices are in the snap's device cgroup"
+    MATCH "c 195:0 rwm" < /sys/fs/cgroup/devices/snap.test-snapd-tools.env/devices.list
+
+    echo "But nonexisting nvidia devices are not"
+    MATCH -v "c 195:254 rwm" < /sys/fs/cgroup/devices/snap.test-snapd-tools.env/devices.list
+
     # TODO: check device unassociated after removing the udev file and rebooting