path: root/0001-cmd-snap-confine-do-not-include-libglvnd-libraries-f.patch

From f4cefc704d6c46f204b0a0651379e0766d478ba5 Mon Sep 17 00:00:00 2001
Message-Id: <f4cefc704d6c46f204b0a0651379e0766d478ba5.1638862176.git.maciej.zenon.borzecki@canonical.com>
From: James Henstridge <james@jamesh.id.au>
Date: Thu, 2 Dec 2021 17:39:04 +0800
Subject: [PATCH] cmd/snap-confine: do not include libglvnd libraries from the
 host system

* cmd/snap-confine: do not include libglvnd libraries from the host system

* tests: we no longer symlink libGLX.so

* cmd/snap-confine: include glvnd globs for old "base: core" snaps

Ubuntu 16.04 did not include the glvnd driver multiplexing libraries,
and the Mesa version of libGL will not function with the Nvidia X
drivers.

While the glvnd drivers may not be compatible with the libraries in the
snap's sandbox (e.g. we know that Ubuntu 21.10's libEGL uses new glibc
symbols), it is better than the nothing working. In particular, X11
based OpenGL apps will function, which will cover the majority of these
old snaps.

* tests: adjust opengl-nvidia test to use its own test snap.

Also update to run on Ubuntu 20.04, and drop 14.04. Still todo: test
against the core20 test snap.

* cmd/libsnap-confine-private: add a unit test for sc_cleanup_shallow_strv

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>

* tests: don't specify base for gl-core16 test snap

* tests: show that host system GLVND libraries are not exposed to snaps using newer bases

* tests: do not run the gl-core20 part of the test on i386

* tests: show that the nvidia backend drivers are still available on core20

* tests: adjust spread test

* tests: add back missing canary file

* cmd/snap-confine: only create the globs array if NVIDIA_BIARCH||NVIDIA_MULTIARCH

Co-authored-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
---
 .../cleanup-funcs-test.c                      | 22 ++++++
 cmd/libsnap-confine-private/cleanup-funcs.c   |  8 ++
 cmd/libsnap-confine-private/cleanup-funcs.h   | 10 +++
 cmd/snap-confine/mount-support-nvidia.c       | 74 +++++++++++++------
 cmd/snap-confine/mount-support-nvidia.h       |  2 +-
 cmd/snap-confine/mount-support.c              |  2 +-
 .../gl-core16/bin/run                         |  3 +
 .../gl-core16/meta/snap.yaml                  |  9 +++
 .../gl-core20/bin/run                         |  3 +
 .../gl-core20/meta/snap.yaml                  | 10 +++
 tests/main/interfaces-opengl-nvidia/task.yaml | 54 +++++++++-----
 11 files changed, 157 insertions(+), 40 deletions(-)
 create mode 100755 tests/main/interfaces-opengl-nvidia/gl-core16/bin/run
 create mode 100644 tests/main/interfaces-opengl-nvidia/gl-core16/meta/snap.yaml
 create mode 100755 tests/main/interfaces-opengl-nvidia/gl-core20/bin/run
 create mode 100644 tests/main/interfaces-opengl-nvidia/gl-core20/meta/snap.yaml

diff --git a/cmd/libsnap-confine-private/cleanup-funcs-test.c b/cmd/libsnap-confine-private/cleanup-funcs-test.c
index 203193e47792e2e0833b36f51515764ae0c7807c..509df3ee8a50b24a48dad0871495cfebe56b7aef 100644
--- a/cmd/libsnap-confine-private/cleanup-funcs-test.c
+++ b/cmd/libsnap-confine-private/cleanup-funcs-test.c
@@ -142,6 +142,27 @@ static void test_cleanup_close(void)
 	g_assert_cmpint(fd, ==, -1);
 }
 
+static void test_cleanup_shallow_strv(void)
+{
+	/* It is safe to use with a NULL pointer */
+	sc_cleanup_shallow_strv(NULL);
+
+	const char **argses = NULL;
+	/* It is ok of the pointer value is NULL */
+	sc_cleanup_shallow_strv(&argses);
+	g_assert_null(argses);
+
+	argses = calloc(10, sizeof(char *));
+	g_assert_nonnull(argses);
+	/* Fill with bogus pointers so attempts to free them would segfault */
+	for (int i = 0; i < 10; i++) {
+		argses[i] = (char *)0x100 + i;
+	}
+	sc_cleanup_shallow_strv(&argses);
+	g_assert_null(argses);
+	/* If we are alive at this point, most likely only the array was free'd */
+}
+
 static void __attribute__((constructor)) init(void)
 {
 	g_test_add_func("/cleanup/sanity", test_cleanup_sanity);
@@ -150,4 +171,5 @@ static void __attribute__((constructor)) init(void)
 	g_test_add_func("/cleanup/endmntent", test_cleanup_endmntent);
 	g_test_add_func("/cleanup/closedir", test_cleanup_closedir);
 	g_test_add_func("/cleanup/close", test_cleanup_close);
+	g_test_add_func("/cleanup/shallow_strv", test_cleanup_shallow_strv);
 }
diff --git a/cmd/libsnap-confine-private/cleanup-funcs.c b/cmd/libsnap-confine-private/cleanup-funcs.c
index 369235cbcc17426372427d952b7f83a16515c268..d96a2ba0f3e4c5f80bb9b7fe9e699b0262508814 100644
--- a/cmd/libsnap-confine-private/cleanup-funcs.c
+++ b/cmd/libsnap-confine-private/cleanup-funcs.c
@@ -28,6 +28,14 @@ void sc_cleanup_string(char **ptr)
 	}
 }
 
+void sc_cleanup_shallow_strv(const char ***ptr)
+{
+	if (ptr != NULL && *ptr != NULL) {
+		free(*ptr);
+		*ptr = NULL;
+	}
+}
+
 void sc_cleanup_file(FILE ** ptr)
 {
 	if (ptr != NULL && *ptr != NULL) {
diff --git a/cmd/libsnap-confine-private/cleanup-funcs.h b/cmd/libsnap-confine-private/cleanup-funcs.h
index b1fee959c5920d59f67e417795e9e8441378d5e2..43ef1515c9cf8ca0c06abbe051d6c8e73261c92f 100644
--- a/cmd/libsnap-confine-private/cleanup-funcs.h
+++ b/cmd/libsnap-confine-private/cleanup-funcs.h
@@ -40,6 +40,16 @@
  **/
 void sc_cleanup_string(char **ptr);
 
+/**
+ * Shallow free a dynamically allocated string vector.
+ *
+ * The strings in the vector will not be freed.
+ * This function is designed to be used with SC_CLEANUP() macro.
+ * The variable MUST be initialized for correct operation.
+ * The safe initialisation value is NULL.
+ */
+void sc_cleanup_shallow_strv(const char ***ptr);
+
 /**
  * Close an open file.
  *
diff --git a/cmd/snap-confine/mount-support-nvidia.c b/cmd/snap-confine/mount-support-nvidia.c
index 2968e1f21a44c2c9d5ba2698afcb525ae91da7fc..75f7265f1bde49beac5725f3b2bf2a755479476c 100644
--- a/cmd/snap-confine/mount-support-nvidia.c
+++ b/cmd/snap-confine/mount-support-nvidia.c
@@ -81,19 +81,10 @@ static const size_t egl_vendor_globs_len =
 // FIXME: this doesn't yet work with libGLX and libglvnd redirector
 // FIXME: this still doesn't work with the 361 driver
 static const char *nvidia_globs[] = {
-	"libEGL.so*",
 	"libEGL_nvidia.so*",
-	"libGL.so*",
-	"libOpenGL.so*",
-	"libGLESv1_CM.so*",
 	"libGLESv1_CM_nvidia.so*",
-	"libGLESv2.so*",
 	"libGLESv2_nvidia.so*",
-	"libGLX_indirect.so*",
 	"libGLX_nvidia.so*",
-	"libGLX.so*",
-	"libGLdispatch.so*",
-	"libGLU.so*",
 	"libXvMCNVIDIA.so*",
 	"libXvMCNVIDIA_dynamic.so*",
 	"libnvidia-cfg.so*",
@@ -162,6 +153,21 @@ static const char *nvidia_globs[] = {
 static const size_t nvidia_globs_len =
     sizeof nvidia_globs / sizeof *nvidia_globs;
 
+static const char *glvnd_globs[] = {
+	"libEGL.so*",
+	"libGL.so*",
+	"libOpenGL.so*",
+	"libGLESv1_CM.so*",
+	"libGLESv2.so*",
+	"libGLX_indirect.so*",
+	"libGLX.so*",
+	"libGLdispatch.so*",
+	"libGLU.so*",
+};
+
+static const size_t glvnd_globs_len =
+    sizeof glvnd_globs / sizeof *glvnd_globs;
+
 #endif				// defined(NVIDIA_BIARCH) || defined(NVIDIA_MULTIARCH)
 
 // Populate libgl_dir with a symlink farm to files matching glob_list.
@@ -351,7 +357,7 @@ static void sc_mkdir_and_mount_and_glob_files(const char *rootfs_dir,
 //
 // In non GLVND cases we just copy across the exposed libGLs and NVIDIA
 // libraries from wherever we find, and clobbering is also harmless.
-static void sc_mount_nvidia_driver_biarch(const char *rootfs_dir)
+static void sc_mount_nvidia_driver_biarch(const char *rootfs_dir, const char **globs, size_t globs_len)
 {
 
 	const char *native_sources[] = {
@@ -374,14 +380,14 @@ static void sc_mount_nvidia_driver_biarch(const char *rootfs_dir)
 	// Primary arch
 	sc_mkdir_and_mount_and_glob_files(rootfs_dir,
 					  native_sources, native_sources_len,
-					  SC_LIBGL_DIR, nvidia_globs,
-					  nvidia_globs_len);
+					  SC_LIBGL_DIR, globs,
+					  globs_len);
 
 #if UINTPTR_MAX == 0xffffffffffffffff
 	// Alternative 32-bit support
 	sc_mkdir_and_mount_and_glob_files(rootfs_dir, lib32_sources,
 					  lib32_sources_len, SC_LIBGL32_DIR,
-					  nvidia_globs, nvidia_globs_len);
+					  globs, globs_len);
 #endif
 }
 
@@ -501,7 +507,7 @@ static int sc_mount_nvidia_is_driver_in_dir(const char *dir)
 	return 0;
 }
 
-static void sc_mount_nvidia_driver_multiarch(const char *rootfs_dir)
+static void sc_mount_nvidia_driver_multiarch(const char *rootfs_dir, const char **globs, size_t globs_len)
 {
 	const char *native_libdir = NATIVE_LIBDIR "/" HOST_ARCH_TRIPLET;
 	const char *lib32_libdir = NATIVE_LIBDIR "/" HOST_ARCH32_TRIPLET;
@@ -519,8 +525,8 @@ static void sc_mount_nvidia_driver_multiarch(const char *rootfs_dir)
 		sc_mkdir_and_mount_and_glob_files(rootfs_dir,
 						  native_sources,
 						  native_sources_len,
-						  SC_LIBGL_DIR, nvidia_globs,
-						  nvidia_globs_len);
+						  SC_LIBGL_DIR, globs,
+						  globs_len);
 
 		// Alternative 32-bit support
 		if ((strlen(HOST_ARCH32_TRIPLET) > 0) &&
@@ -536,8 +542,8 @@ static void sc_mount_nvidia_driver_multiarch(const char *rootfs_dir)
 							  lib32_sources,
 							  lib32_sources_len,
 							  SC_LIBGL32_DIR,
-							  nvidia_globs,
-							  nvidia_globs_len);
+							  globs,
+							  globs_len);
 		}
 	} else {
 		// Attempt mount of both the native and 32-bit variants of the driver if they exist
@@ -576,7 +582,7 @@ static void sc_mount_egl(const char *rootfs_dir)
 					  egl_vendor_globs_len);
 }
 
-void sc_mount_nvidia_driver(const char *rootfs_dir)
+void sc_mount_nvidia_driver(const char *rootfs_dir, const char *base_snap_name)
 {
 	/* If NVIDIA module isn't loaded, don't attempt to mount the drivers */
 	if (access(SC_NVIDIA_DRIVER_VERSION_FILE, F_OK) != 0) {
@@ -593,11 +599,37 @@ void sc_mount_nvidia_driver(const char *rootfs_dir)
 		die("cannot change ownership of " SC_LIB);
 	}
 	(void)sc_set_effective_identity(old);
+
+#if defined(NVIDIA_BIARCH) || defined(NVIDIA_MULTIARCH)
+	/* We include the globs for the glvnd libraries for old snaps
+	 * based on core, Ubuntu 16.04 did not include glvnd itself.
+	 *
+	 * While there is no guarantee that the host system's glvnd
+	 * libGL will be compatible (as it is built with the host
+	 * system's glibc), the Mesa libGL included with the snap will
+	 * definitely not be compatible (as it expects to find the Mesa
+	 * implementation of the GLX extension)..
+	 */
+	const char **globs = nvidia_globs;
+	size_t globs_len = nvidia_globs_len;
+	const char **full_globs SC_CLEANUP(sc_cleanup_shallow_strv) = NULL;
+	if (sc_streq(base_snap_name, "core")) {
+		full_globs = malloc(sizeof nvidia_globs + sizeof glvnd_globs);
+		if (full_globs == NULL) {
+			die("cannot allocate globs array");
+		}
+		memcpy(full_globs, nvidia_globs, sizeof nvidia_globs);
+		memcpy(&full_globs[nvidia_globs_len], glvnd_globs, sizeof glvnd_globs);
+		globs = full_globs;
+		globs_len = nvidia_globs_len + glvnd_globs_len;
+	}
+#endif
+
 #ifdef NVIDIA_MULTIARCH
-	sc_mount_nvidia_driver_multiarch(rootfs_dir);
+	sc_mount_nvidia_driver_multiarch(rootfs_dir, globs, globs_len);
 #endif				// ifdef NVIDIA_MULTIARCH
 #ifdef NVIDIA_BIARCH
-	sc_mount_nvidia_driver_biarch(rootfs_dir);
+	sc_mount_nvidia_driver_biarch(rootfs_dir, globs, globs_len);
 #endif				// ifdef NVIDIA_BIARCH
 
 	// Common for both driver mechanisms
diff --git a/cmd/snap-confine/mount-support-nvidia.h b/cmd/snap-confine/mount-support-nvidia.h
index 56ec893f6c7d8c1cecfd0a16c17add2540bfb32b..9835fb42665b1e2c65a9b557c81e73e2f296aceb 100644
--- a/cmd/snap-confine/mount-support-nvidia.h
+++ b/cmd/snap-confine/mount-support-nvidia.h
@@ -43,6 +43,6 @@
  * /usr/lib directory on the classic filesystem. After the pivot_root() call
  * those symlinks rely on the /var/lib/snapd/hostfs directory as a "gateway".
  **/
-void sc_mount_nvidia_driver(const char *rootfs_dir);
+void sc_mount_nvidia_driver(const char *rootfs_dir, const char *base_snap_name);
 
 #endif
diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c
index 44dea9d9550b047f169b47647276c73425e4f7b6..d5331b2eebb1e612955131f66b6be7f6f217da15 100644
--- a/cmd/snap-confine/mount-support.c
+++ b/cmd/snap-confine/mount-support.c
@@ -494,7 +494,7 @@ static void sc_bootstrap_mount_namespace(const struct sc_mount_config *config)
 	// code changes the nvidia code assumes it has access to the existing
 	// pre-pivot filesystem.
 	if (config->distro == SC_DISTRO_CLASSIC) {
-		sc_mount_nvidia_driver(scratch_dir);
+		sc_mount_nvidia_driver(scratch_dir, config->base_snap_name);
 	}
 	// XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 	//                    pivot_root
diff --git a/tests/main/interfaces-opengl-nvidia/gl-core16/bin/run b/tests/main/interfaces-opengl-nvidia/gl-core16/bin/run
new file mode 100755
index 0000000000000000000000000000000000000000..f07e1ec43b397bf78af6a20ab96a3d4cee87317f
--- /dev/null
+++ b/tests/main/interfaces-opengl-nvidia/gl-core16/bin/run
@@ -0,0 +1,3 @@
+#!/bin/sh
+PS1='$ '
+exec "$@"
diff --git a/tests/main/interfaces-opengl-nvidia/gl-core16/meta/snap.yaml b/tests/main/interfaces-opengl-nvidia/gl-core16/meta/snap.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..83851c75ef120a19d186c25ad421d5a06d6a876a
--- /dev/null
+++ b/tests/main/interfaces-opengl-nvidia/gl-core16/meta/snap.yaml
@@ -0,0 +1,9 @@
+name: gl-core16
+version: 1.0
+summary: Test snap that plugs opengl and uses the core base snap
+confinement: strict
+
+apps:
+  gl-core16:
+    command: bin/run
+    plugs: [ opengl ]
diff --git a/tests/main/interfaces-opengl-nvidia/gl-core20/bin/run b/tests/main/interfaces-opengl-nvidia/gl-core20/bin/run
new file mode 100755
index 0000000000000000000000000000000000000000..f07e1ec43b397bf78af6a20ab96a3d4cee87317f
--- /dev/null
+++ b/tests/main/interfaces-opengl-nvidia/gl-core20/bin/run
@@ -0,0 +1,3 @@
+#!/bin/sh
+PS1='$ '
+exec "$@"
diff --git a/tests/main/interfaces-opengl-nvidia/gl-core20/meta/snap.yaml b/tests/main/interfaces-opengl-nvidia/gl-core20/meta/snap.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..422f183b8316d0dd32bac76e9d83fdc953c02645
--- /dev/null
+++ b/tests/main/interfaces-opengl-nvidia/gl-core20/meta/snap.yaml
@@ -0,0 +1,10 @@
+name: gl-core20
+version: 1.0
+summary: Test snap that plugs opengl and uses the core20 base snap
+confinement: strict
+base: core20
+
+apps:
+  gl-core20:
+    command: bin/run
+    plugs: [ opengl ]
diff --git a/tests/main/interfaces-opengl-nvidia/task.yaml b/tests/main/interfaces-opengl-nvidia/task.yaml
index 0767c587b4f8231c0deb7df4236de32ebc84e788..bad3ed4e3acfb8f603d676d6ab66a5584e3767b3 100644
--- a/tests/main/interfaces-opengl-nvidia/task.yaml
+++ b/tests/main/interfaces-opengl-nvidia/task.yaml
@@ -1,6 +1,6 @@
 summary: Ensure that basic opengl works with faked nvidia
 
-systems: [ubuntu-14.04-*, ubuntu-16.04-*, ubuntu-18.04-*]
+systems: [ubuntu-16.04-*, ubuntu-18.04-*, ubuntu-20.04-*]
 
 environment:
     NV_VERSION/stable: "123.456"
@@ -18,7 +18,7 @@ prepare: |
     mkdir -p /usr/share/vulkan/icd.d
     echo "canary-vulkan" > /usr/share/vulkan/icd.d/nvidia_icd.json
 
-    if os.query is-bionic; then
+    if ! os.query is-xenial; then
         # mock GLVND EGL vendor file
         echo "Test GLVND EGL vendor files access"
         mkdir -p /usr/share/glvnd/egl_vendor.d
@@ -26,7 +26,7 @@ prepare: |
     fi
 
     # mock nvidia libraries
-    if os.query is-bionic; then
+    if ! os.query is-xenial; then
         mkdir -p /usr/lib/"$(dpkg-architecture -qDEB_HOST_MULTIARCH)"/tls
         mkdir -p /usr/lib/"$(dpkg-architecture -qDEB_HOST_MULTIARCH)"/vdpau
         echo "canary-triplet" >> /usr/lib/"$(dpkg-architecture -qDEB_HOST_MULTIARCH)"/libGLX.so.0.0.1
@@ -69,7 +69,7 @@ restore: |
     umount -t tmpfs /sys/module
     rm -rf /usr/share/vulkan
 
-    if os.query is-bionic; then
+    if ! os.query is-xenial; then
         rm -rf /usr/share/glvnd/egl_vendor.d/10_nvidia.json
         rm -rf /usr/lib/"$(dpkg-architecture -qDEB_HOST_MULTIARCH)"/tls
         rm -rf /usr/lib/"$(dpkg-architecture -qDEB_HOST_MULTIARCH)"/vdpau
@@ -90,35 +90,55 @@ restore: |
     rm -rf /usr/lib32/nvidia-123
 
 execute: |
-    "$TESTSTOOLS"/snaps-state install-local test-snapd-policy-app-consumer
+    "$TESTSTOOLS"/snaps-state install-local gl-core16
 
     echo "When the interface is connected"
-    snap connect test-snapd-policy-app-consumer:opengl core:opengl
+    snap connect gl-core16:opengl core:opengl
 
     echo "App can access nvidia library files"
-    expected="canary-legacy"
-    if os.query is-bionic; then
-        expected="canary-triplet"
+    expected="canary-triplet"
+    if os.query is-xenial; then
+        expected="canary-legacy"
     fi
     files="libGLX.so.0.0.1 libGLX_nvidia.so.0.0.1 libnvidia-glcore.so.$NV_VERSION tls/libnvidia-tls.so.$NV_VERSION libnvidia-tls.so.$NV_VERSION vdpau/libvdpau_nvidia.so.$NV_VERSION"
     for f in $files; do
-       snap run test-snapd-policy-app-consumer.opengl -c "cat /var/lib/snapd/lib/gl/$f" | MATCH "$expected"
+       gl-core16 cat "/var/lib/snapd/lib/gl/$f" | MATCH "$expected"
     done
 
     if os.query is-pc-amd64; then
-        expected32="canary-32-legacy"
-        if os.query is-bionic; then
-            expected32="canary-32-triplet"
+        expected32="canary-32-triplet"
+        if os.query is-xenial; then
+            expected32="canary-32-legacy"
         fi
         for f in $files; do
-            snap run test-snapd-policy-app-consumer.opengl -c "cat /var/lib/snapd/lib/gl32/$f" | MATCH "$expected32"
+            gl-core16 cat "/var/lib/snapd/lib/gl32/$f" | MATCH "$expected32"
         done
     fi
 
     echo "And vulkan ICD file"
-    snap run test-snapd-policy-app-consumer.opengl -c "cat /var/lib/snapd/lib/vulkan/icd.d/nvidia_icd.json" | MATCH canary-vulkan
+    gl-core16 cat /var/lib/snapd/lib/vulkan/icd.d/nvidia_icd.json | MATCH canary-vulkan
 
-    if os.query is-bionic; then
+    if ! os.query is-xenial; then
         echo "And GLVND EGL vendor file"
-        snap run test-snapd-policy-app-consumer.opengl -c "cat /var/lib/snapd/lib/glvnd/egl_vendor.d/10_nvidia.json" | MATCH canary-egl
+        gl-core16 cat /var/lib/snapd/lib/glvnd/egl_vendor.d/10_nvidia.json | MATCH canary-egl
+    fi
+
+    # There is no core20 snap on i386, so the following tests will not
+    # function there.
+    if os.query is-pc-i386; then
+        exit 0
+    fi
+
+    echo "For host systems using glvnd, the glvnd libraries are not exposed to snaps using newer bases"
+    "$TESTSTOOLS"/snaps-state install-local gl-core20
+    snap connect gl-core20:opengl core:opengl
+
+    echo "While glvnd frontend libraries are not available, the backend nvidia drivers are"
+    if ! os.query is-xenial; then
+        not gl-core20 test -f /var/lib/snapd/lib/gl/libGLX.so.0.0.1
+        gl-core20 cat /var/lib/snapd/lib/gl/libGLX_nvidia.so.0.0.1 | MATCH canary-triplet
+        if os.query is-pc-amd64; then
+            not gl-core20 cat /var/lib/snapd/lib/gl32/libGLX.so.0.0.1
+            gl-core20 cat /var/lib/snapd/lib/gl32/libGLX_nvidia.so.0.0.1 | MATCH canary-32-triplet
+        fi
     fi
-- 
2.34.1