[libvirt] [PATCH 2/2] security: aa-helper: gl devices in sysfs at arbitrary depth

[Christian Ehrhardt]

On Tue, Mar 5, 2019 at 5:48 PM Jamie Strandboge <jamie at 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 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 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


> --
> Jamie Strandboge             | http://www.canonical.com



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd