[Pkg-libvirt-commits] [Git][libvirt-team/libvirt][debian/sid] Fix virtio-gpu + virgl support by cherry-picking upstream commits

Guido Günther gitlab at salsa.debian.org
Mon Apr 1 10:31:47 BST 2019



Guido Günther pushed to branch debian/sid at Libvirt Packaging Team / libvirt


Commits:
d7a7218f by intrigeri at 2019-04-01T09:30:52Z
Fix virtio-gpu + virgl support by cherry-picking upstream commits

virt-manager in current sid still creates new VMs with QXL graphics by default,
so this bug only affects users who opt in for virtio-gpu + 3D acceleration.
Still, the option for virtio-gpu + 3D acceleration is offered in the
virt-manager GUI, so having it broken by default is an important problem.

These are upstream commits:
f2cbb94eabdd5e3422c45b1afa48eb4c951c09e0
00fbb9e51678f76effa2d20e78a9be861ad5f484
27a9ebf28183cb3c3c784fcab622e67e978eb3dc
d85e8e400b48f1b4c1dfbf438dda83cd959eacf7
fb01e1a44daea773cd53f275cad6f031506c20db

Closes: #916587

- - - - -


6 changed files:

- + debian/patches/security-aa-helper-allow-virt-aa-helper-to-read-dev-dri.patch
- + debian/patches/security-aa-helper-generate-more-rules-for-gl-devices.patch
- + debian/patches/security-aa-helper-gl-devices-in-sysfs-at-arbitrary-depth.patch
- + debian/patches/security-aa-helper-nvidia-rules-for-gl-devices.patch
- debian/patches/series
- + debian/patches/virt-aa-helper-generate-rules-for-gl-enabled-graphics-dev.patch


Changes:

=====================================
debian/patches/security-aa-helper-allow-virt-aa-helper-to-read-dev-dri.patch
=====================================
@@ -0,0 +1,33 @@
+From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+Date: Tue, 12 Feb 2019 10:33:23 +0100
+Subject: security: aa-helper: allow virt-aa-helper to read /dev/dri
+
+Change fb01e1a44 "virt-aa-helper: generate rules for gl enabled
+graphics devices" implemented the detection for gl enabled
+devices in virt-aa-helper. But it will in certain cases e.g. if
+no rendernode was explicitly specified need to read /dev/dri
+which it currently isn't allowed.
+
+Add a rule to the apparmor profile of virt-aa-helper itself to
+be able to do that.
+
+Acked-by: Jamie Strandboge <jamie at canonical.com>
+Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+---
+ src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper
+index 665094a..2d43057 100644
+--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper
++++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper
+@@ -20,6 +20,9 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper {
+ 
+   /etc/libnl-3/classid r,
+ 
++  # for gl enabled graphics
++  /dev/dri/{,*} r,
++
+   # for hostdev
+   /sys/devices/ r,
+   /sys/devices/** r,


=====================================
debian/patches/security-aa-helper-generate-more-rules-for-gl-devices.patch
=====================================
@@ -0,0 +1,82 @@
+From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+Date: Tue, 12 Feb 2019 11:12:52 +0100
+Subject: security: aa-helper: generate more rules for gl devices
+
+Change fb01e1a44 "virt-aa-helper: generate rules for gl enabled
+graphics devices" implemented the detection for gl enabled
+devices in virt-aa-helper. But further testing showed
+that it will need much more access for the full gl stack
+to work.
+
+Upstream apparmor just recently split those things out and now
+has two related abstractions at
+https://gitlab.com/apparmor/apparmor/blob/master:
+- dri-common at /profiles/apparmor.d/abstractions/dri-common
+- mesa: at /profiles/apparmor.d/abstractions/mesa
+
+If would be great to just include that for the majority of
+rules, but they are not yet in any distribution so we need
+to add rules inspired by them based on the testing that we
+can do.
+
+Furthermore qemu with opengl will also probe the backing device
+of the rendernode for attributes which should be safe as
+read-only wildcard rules.
+
+Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1815452
+
+Acked-by: Jamie Strandboge <jamie at canonical.com>
+Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+---
+ src/security/virt-aa-helper.c | 21 ++++++++++++++++++++-
+ 1 file changed, 20 insertions(+), 1 deletion(-)
+
+diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
+index 46443a3..cc8a327 100644
+--- a/src/security/virt-aa-helper.c
++++ b/src/security/virt-aa-helper.c
+@@ -938,7 +938,7 @@ get_files(vahControl * ctl)
+     size_t i;
+     char *uuid;
+     char uuidstr[VIR_UUID_STRING_BUFLEN];
+-    bool needsVfio = false, needsvhost = false;
++    bool needsVfio = false, needsvhost = false, needsgl = false;
+ 
+     /* verify uuid is same as what we were given on the command line */
+     virUUIDFormat(ctl->def->uuid, uuidstr);
+@@ -1066,9 +1066,11 @@ get_files(vahControl * ctl)
+ 
+         if (rendernode) {
+             vah_add_file(&buf, rendernode, "rw");
++            needsgl = true;
+         } else {
+             if (virDomainGraphicsNeedsAutoRenderNode(graphics)) {
+                 char *defaultRenderNode = virHostGetDRMRenderNode();
++                needsgl = true;
+ 
+                 if (defaultRenderNode) {
+                     vah_add_file(&buf, defaultRenderNode, "rw");
+@@ -1268,6 +1270,23 @@ get_files(vahControl * ctl)
+         virBufferAddLit(&buf, "  \"/dev/vfio/vfio\" rw,\n");
+         virBufferAddLit(&buf, "  \"/dev/vfio/[0-9]*\" rw,\n");
+     }
++    if (needsgl) {
++        /* if using gl all sorts of further dri related paths will be needed */
++        virBufferAddLit(&buf, "  # DRI/Mesa/(e)GL config and driver paths\n");
++        virBufferAddLit(&buf, "  \"/usr/lib{,32,64}/dri/*.so*\" mr,\n");
++        virBufferAddLit(&buf, "  \"/usr/lib/@{multiarch}/dri/*.so*\" mr,\n");
++        virBufferAddLit(&buf, "  \"/usr/lib/fglrx/dri/*.so*\" mr,\n");
++        virBufferAddLit(&buf, "  \"/etc/drirc\" r,\n");
++        virBufferAddLit(&buf, "  \"/usr/share/drirc.d/{,*.conf}\" r,\n");
++        virBufferAddLit(&buf, "  \"/etc/glvnd/egl_vendor.d/{,*}\" r,\n");
++        virBufferAddLit(&buf, "  \"/usr/share/glvnd/egl_vendor.d/{,*}\" r,\n");
++        virBufferAddLit(&buf, "  # Probe DRI device attributes\n");
++        virBufferAddLit(&buf, "  \"/dev/dri/\" r,\n");
++        virBufferAddLit(&buf, "  \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
++        virBufferAddLit(&buf, "  \"/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
++        virBufferAddLit(&buf, "  # dri libs will trigger that, but t is not requited and DAC would deny it anyway\n");
++        virBufferAddLit(&buf, "  deny \"/var/lib/libvirt/.cache/\" w,\n");
++    }
+ 
+     if (ctl->newfile)
+         if (vah_add_file(&buf, ctl->newfile, "rwk") != 0)


=====================================
debian/patches/security-aa-helper-gl-devices-in-sysfs-at-arbitrary-depth.patch
=====================================
@@ -0,0 +1,43 @@
+From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+Date: Tue, 5 Mar 2019 13:38:38 +0100
+Subject: security: aa-helper: gl devices in sysfs at arbitrary depth
+
+Further testing with more devices showed that we sometimes have a
+different depth of pci device paths when accessing sysfs for device
+attributes.
+
+But since the access is limited to a set of filenames and read only it
+is safe to use a wildcard for that.
+
+Related apparmor denies - while we formerly had only considered:
+apparmor="DENIED" operation="open"
+  name="/sys/devices/pci0000:00/0000:00:02.1/uevent"
+  requested_mask="r"
+
+We now also know of cases like:
+apparmor="DENIED" operation="open"
+  name="/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/uevent"
+  requested_mask="r"
+
+Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
+
+Acked-by: Jamie Strandboge <jamie at canonical.com>
+Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+---
+ src/security/virt-aa-helper.c | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
+index c34afc8..2dc68b2 100644
+--- a/src/security/virt-aa-helper.c
++++ b/src/security/virt-aa-helper.c
+@@ -1287,8 +1287,7 @@ get_files(vahControl * ctl)
+         virBufferAddLit(&buf, "  \"/dev/nvidiactl\" rw,\n");
+         virBufferAddLit(&buf, "  # Probe DRI device attributes\n");
+         virBufferAddLit(&buf, "  \"/dev/dri/\" r,\n");
+-        virBufferAddLit(&buf, "  \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
+-        virBufferAddLit(&buf, "  \"/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
++        virBufferAddLit(&buf, "  \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
+         virBufferAddLit(&buf, "  # dri libs will trigger that, but t is not requited and DAC would deny it anyway\n");
+         virBufferAddLit(&buf, "  deny \"/var/lib/libvirt/.cache/\" w,\n");
+     }


=====================================
debian/patches/security-aa-helper-nvidia-rules-for-gl-devices.patch
=====================================
@@ -0,0 +1,44 @@
+From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+Date: Fri, 1 Mar 2019 07:25:59 +0100
+Subject: security: aa-helper: nvidia rules for gl devices
+
+Further testing with different devices showed that we need more rules
+to drive gl backends with nvidia cards. Related denies look like:
+
+apparmor="DENIED" operation="open"
+  name="/usr/share/egl/egl_external_platform.d/"
+  requested_mask="r"
+apparmor="DENIED" operation="open"
+  name="/proc/modules"
+  requested_mask="r"
+apparmor="DENIED" operation="open"
+  name="/proc/driver/nvidia/params"
+  requested_mask="r"
+apparmor="DENIED" operation="mknod"
+  name="/dev/nvidiactl"
+  requested_mask="c"
+
+Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
+
+Acked-by: Jamie Strandboge <jamie at canonical.com>
+Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+---
+ src/security/virt-aa-helper.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
+index cc8a327..c34afc8 100644
+--- a/src/security/virt-aa-helper.c
++++ b/src/security/virt-aa-helper.c
+@@ -1280,6 +1280,11 @@ get_files(vahControl * ctl)
+         virBufferAddLit(&buf, "  \"/usr/share/drirc.d/{,*.conf}\" r,\n");
+         virBufferAddLit(&buf, "  \"/etc/glvnd/egl_vendor.d/{,*}\" r,\n");
+         virBufferAddLit(&buf, "  \"/usr/share/glvnd/egl_vendor.d/{,*}\" r,\n");
++        virBufferAddLit(&buf, "  \"/usr/share/egl/egl_external_platform.d/\" r,\n");
++        virBufferAddLit(&buf, "  \"/usr/share/egl/egl_external_platform.d/*\" r,\n");
++        virBufferAddLit(&buf, "  \"/proc/modules\" r,\n");
++        virBufferAddLit(&buf, "  \"/proc/driver/nvidia/params\" r,\n");
++        virBufferAddLit(&buf, "  \"/dev/nvidiactl\" rw,\n");
+         virBufferAddLit(&buf, "  # Probe DRI device attributes\n");
+         virBufferAddLit(&buf, "  \"/dev/dri/\" r,\n");
+         virBufferAddLit(&buf, "  \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");


=====================================
debian/patches/series
=====================================
@@ -14,3 +14,8 @@ Set-defaults-for-zfs-tools.patch
 Pass-GPG_TTY-env-var-to-the-ssh-binary.patch
 apparmor-Allow-virt-aa-helper-to-access-the-name-service-.patch
 debian/Prefer-sbin-over-usr-sbin.patch
+virt-aa-helper-generate-rules-for-gl-enabled-graphics-dev.patch
+security-aa-helper-allow-virt-aa-helper-to-read-dev-dri.patch
+security-aa-helper-generate-more-rules-for-gl-devices.patch
+security-aa-helper-nvidia-rules-for-gl-devices.patch
+security-aa-helper-gl-devices-in-sysfs-at-arbitrary-depth.patch


=====================================
debian/patches/virt-aa-helper-generate-rules-for-gl-enabled-graphics-dev.patch
=====================================
@@ -0,0 +1,73 @@
+From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+Date: Mon, 14 Jan 2019 15:15:06 +0200
+Subject: virt-aa-helper: generate rules for gl enabled graphics devices
+
+This adds the virt-aa-helper support for gl enabled graphics devices to
+generate rules for the needed rendernode paths.
+
+Example in domain xml:
+<graphics type='spice'>
+  <gl enable='yes' rendernode='/dev/dri/bar'/>
+</graphics>
+
+results in:
+  "/dev/dri/bar" rw,
+
+Special cases are:
+- multiple devices with rendernodes -> all are added
+- non explicit rendernodes -> follow recently added virHostGetDRMRenderNode
+- rendernode without opengl (in egl-headless for example) -> still add
+  the node
+
+Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
+
+Reviewed-by: Erik Skultety <eskultet at redhat.com>
+Acked-by: Jamie Strandboge <jamie at canonical.com>
+Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
+---
+ src/security/virt-aa-helper.c | 14 ++++++++++++++
+ tests/virt-aa-helper-test     |  6 ++++++
+ 2 files changed, 20 insertions(+)
+
+diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
+index 64a4256..46443a3 100644
+--- a/src/security/virt-aa-helper.c
++++ b/src/security/virt-aa-helper.c
+@@ -1062,6 +1062,20 @@ get_files(vahControl * ctl)
+     for (i = 0; i < ctl->def->ngraphics; i++) {
+         virDomainGraphicsDefPtr graphics = ctl->def->graphics[i];
+         size_t n;
++        const char *rendernode = virDomainGraphicsGetRenderNode(graphics);
++
++        if (rendernode) {
++            vah_add_file(&buf, rendernode, "rw");
++        } else {
++            if (virDomainGraphicsNeedsAutoRenderNode(graphics)) {
++                char *defaultRenderNode = virHostGetDRMRenderNode();
++
++                if (defaultRenderNode) {
++                    vah_add_file(&buf, defaultRenderNode, "rw");
++                    VIR_FREE(defaultRenderNode);
++                }
++            }
++        }
+ 
+         for (n = 0; n < graphics->nListens; n++) {
+             virDomainGraphicsListenDef listenObj = graphics->listens[n];
+diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
+index fb40057..6e674bf 100755
+--- a/tests/virt-aa-helper-test
++++ b/tests/virt-aa-helper-test
+@@ -378,6 +378,12 @@ testme "0" "input dev passthrough" "-r -u $valid_uuid" "$test_xml" "$disk2.*rw,$
+ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,<memory>524288</memory>,<memory>1048576</memory>,g" -e "s,</devices>,<memory model='nvdimm'><source><path>$disk2</path></source><target><size unit='KiB'>524288</size><node>0</node></target></memory></devices>,g" "$template_xml" > "$test_xml"
+ testme "0" "nvdimm" "-r -u $valid_uuid" "$test_xml" "$disk2.*rw,$"
+ 
++sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<graphics type='egl-headless'><gl rendernode='/dev/dri/testegl1'/></graphics></devices>,g" "$template_xml" > "$test_xml"
++testme "0" "dri egl" "-r -u $valid_uuid" "$test_xml" "/dev/dri/testegl1.*rw,$"
++
++sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<graphics type='spice'><gl enable='yes' rendernode='/dev/dri/testegl2'/></graphics></devices>,g" "$template_xml" > "$test_xml"
++testme "0" "dri spice" "-r -u $valid_uuid" "$test_xml" "/dev/dri/testegl2.*rw,$"
++
+ testme "0" "help" "-h"
+ 
+ echo "" >$output



View it on GitLab: https://salsa.debian.org/libvirt-team/libvirt/commit/d7a7218fe6539b5ebc9c61dd3df6385e82ebbe5d

-- 
View it on GitLab: https://salsa.debian.org/libvirt-team/libvirt/commit/d7a7218fe6539b5ebc9c61dd3df6385e82ebbe5d
You're receiving this email because of your account on salsa.debian.org.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-libvirt-commits/attachments/20190401/99365a59/attachment-0001.html>


More information about the Pkg-libvirt-commits mailing list