On Tue, Mar 5, 2019 at 5:48 PM Jamie Strandboge <jamie(a)canonical.com> wrote:
On Tue, 05 Mar 2019, Christian Ehrhardt wrote:
> 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
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)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 13b507ff69..989dcf1784 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1286,8 +1286,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");
I'm curious about the new denials, but the reads for these files should be
fine.
Yes it is odd, but as it seems to be HW dependent that seems to be the
only way covers what is needed and seems safe.
Thanks for reviewing - Pushed with your ack