OpenGL accel dev work doesn't work on nvidia card

Thanks a lot Brian!

The denies for /sys/devices will be covered by my recent upstream commit which is a bit less "open" :-)

This is stuff we really need to add:
  /usr/share/egl/egl_external_platform.d/ r,
  /usr/share/egl/egl_external_platform.d/** r,
  /proc/modules r,

Less open than you suggested but should work, I recently added upstream:
  /etc/glvnd/egl_vendor.d/{,*} r,
which together with the rule above should be fine.

Note, all that is only done if GL is enabled which makes it rather secure to not open it up in general.

The following rules you added in your test, but I haven't seen the apparmor DENIED examples in any of the logs.
Would you mind running it without those rules and provide those deny example logs for:
  /proc/driver/nvidia/ r,
  /proc/driver/nvidia/** r,
  /dev/nvidiactl rw,

Once I have that I can push a change online extending what I did for i915 with what you identified.

Once we have that I can do an upload to Disco with all of it and we can give it a retry with all the platforms that we have.

Status "Incomplete" for the extra logs on denies on the ...nvidia... paths

Oops, sorry. It's important to note even it still gives me the same error - ../src/gallium/drivers/llvmpipe/lp_texture.c:499.. with or without these nvidia lines. I was not pursing the most restrictive lines, just trying to get it to work.

Feb 28 08:24:29 desktop audit[14202]: AVC apparmor="DENIED" operation="open" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/proc/driver/nvidia/params" pid=14202 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0
Feb 28 08:24:29 desktop audit[14202]: AVC apparmor="DENIED" operation="mknod" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/dev/nvidiactl" pid=14202 comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=64055 ouid=64055
Feb 28 08:24:29 desktop kernel: audit: type=1400 audit(1551371069.858:226): apparmor="DENIED" operation="open" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/proc/driver/nvidia/params" pid=14202 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0
Feb 28 08:24:29 desktop kernel: audit: type=1400 audit(1551371069.858:227): apparmor="DENIED" operation="mknod" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/dev/nvidiactl" pid=14202 comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=64055 ouid=64055
Feb 28 08:24:29 desktop audit[14202]: AVC apparmor="DENIED" operation="open" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/proc/driver/nvidia/params" pid=14202 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0
Feb 28 08:24:29 desktop kernel: audit: type=1400 audit(1551371069.934:228): apparmor="DENIED" operation="open" profile="libvirt-26480e4e-9d51-476e-b329-657b2012c151" name="/proc/driver/nvidia/params" pid=14202 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0

Thanks Bryan,
I'll only go for the formerly mentioned and these on the first try then:
  /proc/driver/nvidia/params r,
  /dev/nvidiactl r,

We might need more further down the road, but since we usually want to start restrictive that will be our start.

I'd wish we can stay away from
  /dev/nvidia* rw,
because that is what we might end up with (/dev/nvidia-uvm, /dev/nvidia[0-9], ...), so lets try with the smallest rule that seems to work for now.

I think I lost it in posting the bug, but currently with even my less restrictive bits, the VM fails to start with the gallium error.

My C might be a little rusty.. but the code in question seems redundant:
assert(lpr->dt);
if (!lpr->dt)
   return FALSE;

https://github.com/mesa3d/mesa/blob/master/src/gallium/drivers/llvmpipe/lp_texture.c

Just removing the assert (somewhat unsurprisingly) isn't enough to make it work.

(Additional apparmor rules)
  /proc/modules r,
  /proc/driver/nvidia/params r,
  /etc/glvnd/egl_vendor.d/{,*} r,
  /usr/share/egl/egl_external_platform.d/ r,
  /usr/share/egl/egl_external_platform.d/** r,
  /sys/devices/** r,
  /sys/devices/ r,
  /dev/nvidiactl rw,

The VM now starts, but displays nothing, and you can get virt-manager to crash with possibly related line (If you have viewer open when you start the VM):
Mar 01 00:14:42 desktop libvirtd[1475]: End of file while reading data: Input/output error

There is (always) hope :-)

Please try the build at PPA [1] if it resolves your apparmor issues.
If it is good I'll submit upstream, if not we will polish the rules as needed.

Furthermore the removal of the assert might in fact be just what you need.
As I expect the remaining error you hit might actually be known and currently discussed in bug 1815889 - feel free to compare your crash with those there and let me know. If it is subscribe there to track once a solution is available.

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3651

Almost there, I still had to do this:
 /sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,
 /sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/drm/renderD128/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,

That is odd, it should have:
/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,
/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device} r

And I'd have thought your remaining rules should be under that:

/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/drm/renderD128/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,

It turns out you have a deeper pathing with one more directory :-/

I wonder if that would be more appropriate:
- /using (**) to get any depth of device cascading in /sys
- since we have ** we can drop the drm as it doesn't matter anymore

/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,

That most likely works, but needs security ack as it is slightly more open.

@Brian - could you use the rule I posted above instead of yours of comment #12 and see if that would work?

/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device} r,
Works (as in crashes), but at least no apparmor deny lines.

Thanks,
lets fix the apparmor denies with that then.
The crashes you encounter will be another discussion and another bug afterwards if you don't mind.

I'll prep a fix for upstream ...

@Brian - I'll provide a new libvirt upload in Disco once I have these things upstream.

Until then you can just go on with the apparmor rules you added manually.
For the crash, could you try it with [1] enabled?
It has an important fix in mesa ro work with qemu gl graphics - not the same you have but still a hard requirement. Since the new mesa stack might change other things it might be worth a try.
If you still crash open a new bug about it stating the libvirt/qemu/mesa/nidia-driver levels you are on - no promise on the speed on that as I lack the HW right now.

[1]: https://launchpad.net/~canonical-x/+archive/ubuntu/x-staging

Submitted upstream as:
https://www.redhat.com/archives/libvir-list/2019-March/msg00218.html
https://www.redhat.com/archives/libvir-list/2019-March/msg00217.html

The changes were not worth a cover letter, but it should work that way as well.
Lets see how the discussion there will end up.

Feel free to grant your ack there after reviewing.
Most of the time we wait at least for one security Team member (mostly Jamie) to say yes as well.

Fix for this and bug 1815452 now uploaded to Disco - lets see how it migrates.

GL seems still shaky in different use cases aside the most default path - therefore I'm not making it easier to use it on older releases - therefore no SRU of the change.

@Brian - after this is ready you might test with this to finally confirm the apparmor piece to be working still as we tested it. If the follow on issue (your crash) still occurs with this and the x-stagign PPA that I linked before please open a new bug for it then.

This bug was fixed in the package libvirt - 5.0.0-1ubuntu2

---------------
libvirt (5.0.0-1ubuntu2) disco; urgency=medium

  * Implement further apparmor rules for usage of gl enabled
    graphics (LP: #1815452)
    - d/p/ubuntu-aa/lp-1815452-more-gl-rules.patch
    - d/p/ubuntu-aa/lp-1815452-virt-aa-helper-rule.patch
  * Implement further apparmor rules for usage of gl enabled
    graphics with nvidia cards (LP: #1817943)
    - d/p/ubuntu-aa/lp-1817943-nvidia-gl-rules.patch
    - d/p/ubuntu-aa/lp-1817943-devices-in-sysfs.patch
  * d/p/ubuntu-aa/lp-1804766-*: updated to the upstream accepted
    version (no functional change, LP: 1804766)

 -- Christian Ehrhardt <email address hidden> Tue, 12 Feb 2019 11:27:14 +0100

Confirmed AppArmor denials are gone in Disco. Will circle back and test x-staging in future.