1. Patchew
  2. Libvirt
  3. View series

[PATCH] Allow VM to read sysfs PCI config, revision files

Max Goodhart posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220511220839.5632-1-c@chromakode.com
src/security/virt-aa-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Max Goodhart 1 year, 11 months ago 
From: Max Goodhart <gitlab@chromakode.com>

This fixes a blank screen when viewing a VM with virtio graphics and
gl-accelerated Spice display on Ubuntu 22.04 / libvirt 8.0.0 / qemu 6.2.

Without these AppArmor permissions, the libvirt error log contains
repetitions of:

qemu_spice_gl_scanout_texture: failed to get fd for texture

This appears to be similar to this GNOME Boxes issue:
https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/586

Signed-off-by: Max Goodhart <c@chromakode.com>
---
 src/security/virt-aa-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 1f1cce8b3d..b314d2a059 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1316,7 +1316,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/**/{uevent,vendor,device,subsystem_vendor,subsystem_device,config,revision}\" 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");
     }
-- 
2.34.1
Re: [PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Christian Ehrhardt 1 year, 11 months ago 
On Thu, May 12, 2022 at 3:27 PM Max Goodhart <c@chromakode.com> wrote:
>
> From: Max Goodhart <gitlab@chromakode.com>

Hi Max,
thanks for the work to identify and fix this!

It is indeed a natural evolution of my 27a9ebf2818 00fbb9e5167
f2cbb94eabd that made the rules so far.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

> This fixes a blank screen when viewing a VM with virtio graphics and
> gl-accelerated Spice display on Ubuntu 22.04 / libvirt 8.0.0 / qemu 6.2.
>
> Without these AppArmor permissions, the libvirt error log contains
> repetitions of:
>
> qemu_spice_gl_scanout_texture: failed to get fd for texture
>
> This appears to be similar to this GNOME Boxes issue:
> https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/586
>
> Signed-off-by: Max Goodhart <c@chromakode.com>
> ---
>  src/security/virt-aa-helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 1f1cce8b3d..b314d2a059 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1316,7 +1316,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/**/{uevent,vendor,device,subsystem_vendor,subsystem_device,config,revision}\" 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");
>      }
> --
> 2.34.1
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Max Goodhart 1 year, 11 months ago 
Oops, I didn't intend for the commit author email to be
gitlab@chromakode.com here. Would you please use c@chromakode.com as the
author of the patch?

On Wed, May 11, 2022, 6:09 PM Max Goodhart <c@chromakode.com> wrote:

> From: Max Goodhart <gitlab@chromakode.com>
>
> This fixes a blank screen when viewing a VM with virtio graphics and
> gl-accelerated Spice display on Ubuntu 22.04 / libvirt 8.0.0 / qemu 6.2.
>
> Without these AppArmor permissions, the libvirt error log contains
> repetitions of:
>
> qemu_spice_gl_scanout_texture: failed to get fd for texture
>
> This appears to be similar to this GNOME Boxes issue:
> https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/586
>
> Signed-off-by: Max Goodhart <c@chromakode.com>
> ---
>  src/security/virt-aa-helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 1f1cce8b3d..b314d2a059 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1316,7 +1316,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/**/{uevent,vendor,device,subsystem_vendor,subsystem_device,config,revision}\"
> 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");
>      }
> --
> 2.34.1
>
>
Re: [PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Christian Ehrhardt 1 year, 11 months ago 
On Thu, May 12, 2022 at 3:27 PM Max Goodhart <c@chromakode.com> wrote:
>
> Oops, I didn't intend for the commit author email to be gitlab@chromakode.com here. Would you please use c@chromakode.com as the author of the patch?

Yeah I can amend that (and we see you control that email address).

Also since I know this is coming from a particular LP bug I'd also add
the following to have the full trace-back from the commit message to
more context

Fixes: https://launchpad.net/bugs/1972075

Can I apply it that way or are there any objections?

> On Wed, May 11, 2022, 6:09 PM Max Goodhart <c@chromakode.com> wrote:
>>
>> From: Max Goodhart <gitlab@chromakode.com>
>>
>> This fixes a blank screen when viewing a VM with virtio graphics and
>> gl-accelerated Spice display on Ubuntu 22.04 / libvirt 8.0.0 / qemu 6.2.
>>
>> Without these AppArmor permissions, the libvirt error log contains
>> repetitions of:
>>
>> qemu_spice_gl_scanout_texture: failed to get fd for texture
>>
>> This appears to be similar to this GNOME Boxes issue:
>> https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/586
>>
>> Signed-off-by: Max Goodhart <c@chromakode.com>
>> ---
>>  src/security/virt-aa-helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
>> index 1f1cce8b3d..b314d2a059 100644
>> --- a/src/security/virt-aa-helper.c
>> +++ b/src/security/virt-aa-helper.c
>> @@ -1316,7 +1316,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/**/{uevent,vendor,device,subsystem_vendor,subsystem_device,config,revision}\" 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");
>>      }
>> --
>> 2.34.1
>>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Michal Prívozník 1 year, 11 months ago 
On 5/19/22 08:09, Christian Ehrhardt wrote:
> On Thu, May 12, 2022 at 3:27 PM Max Goodhart <c@chromakode.com> wrote:
>>
>> Oops, I didn't intend for the commit author email to be gitlab@chromakode.com here. Would you please use c@chromakode.com as the author of the patch?
> 
> Yeah I can amend that (and we see you control that email address).
> 
> Also since I know this is coming from a particular LP bug I'd also add
> the following to have the full trace-back from the commit message to
> more context
> 
> Fixes: https://launchpad.net/bugs/1972075
> 
> Can I apply it that way or are there any objections?

Yeah, go ahead and push. The change does indeed fix the problem.

Michal
Re: [PATCH] Allow VM to read sysfs PCI config, revision files
Posted by Christian Ehrhardt 1 year, 11 months ago 
On Thu, May 19, 2022 at 3:04 PM Michal Prívozník <mprivozn@redhat.com> wrote:
>
> On 5/19/22 08:09, Christian Ehrhardt wrote:
> > On Thu, May 12, 2022 at 3:27 PM Max Goodhart <c@chromakode.com> wrote:
> >>
> >> Oops, I didn't intend for the commit author email to be gitlab@chromakode.com here. Would you please use c@chromakode.com as the author of the patch?
> >
> > Yeah I can amend that (and we see you control that email address).
> >
> > Also since I know this is coming from a particular LP bug I'd also add
> > the following to have the full trace-back from the commit message to
> > more context
> >
> > Fixes: https://launchpad.net/bugs/1972075
> >
> > Can I apply it that way or are there any objections?
>
> Yeah, go ahead and push. The change does indeed fix the problem.

Thanks Michal and Max,
applied!

> Michal
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.