Comment 4 for bug 1686324

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Apparmor rule is:
  "/dev/bus/usb/000/000" rw,

But then while virt-aa-helper is running it really is 0/0
p ctl->def->hostdevs[0]->source.subsys.u.usb
$12 = {autoAddress = false, bus = 0, device = 0, vendor = 1921, product = 21888}

Stack is like this:
  virDomainHostdevSubsysUSBDefParseXML at conf/domain_conf.c:6521
  virDomainHostdevDefParseXMLSubsys conf/domain_conf.c:7213
  virDomainHostdevDefParseXML conf/domain_conf.c:14287
  virDomainDefParseXML conf/domain_conf.c:18968
  virDomainDefParseNode conf/domain_conf.c:19460
  virDomainDefParse conf/domain_conf.c:19404
  virDomainDefParseString conf/domain_conf.c:19420
  get_definition security/virt-aa-helper.c:710

So if the initial XML parsing really only does XML parsing.
Which code fills in the bus/device details.
It does so later on when running before calling virt-aa-helper on the live case.
I confirmed once more that in the live case it correctly creates:
  "/dev/bus/usb/002/011" rw,

At that point (live) it is fully detected, dumpxml confirms:
      <source>
        <vendor id='0x0781'/>
        <product id='0x5580'/>
        <address bus='3' device='2'/>
      </source>

I checked if this might have been a regression of some sort we could find with bisecting.
But even back to trusty with libvirt 1.2.2 this behaves the same way.

There is a set of functions that look good:
virhostdev.c virHostdevFindUSBDevice
virsubusb.c virUSBDeviceFindByVendor, virUSBDeviceFind

But these are only called from virHostdevPrepareUSBDevices which is not on the "parsing" path, but only the actual attaching.

Signature from virhostdev.h:
 virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
                         bool mandatory,
                         virUSBDevicePtr *usb)

To derive the mandatory flag from virt-aa-helper seems to be like guessing at best.
So I set it to false which makes it info-print but not break if it can't find the device.
It would depend on being optional startup policy (that we could check), but we have no way to know if we are coldbooting.
I can decide to make it too strict

Following the thoughts of:
commit e658daeb58ec88deec5dd60fd3279f4feb4eac5b
Author: Jiri Denemark <email address hidden>
Date: Tue Oct 2 15:14:02 2012 +0200

    conf: Add support for startupPolicy for USB devices

    USB devices can disappear without OS being mad about it, which makes
    them ideal for startupPolicy. With this attribute, USB devices can be
    configured to be mandatory (the default), requisite (will disappear
    during migration if they cannot be found), or completely optional.

TL;DR - if we can't find the device we can't add it to the rules.
We do not have to break the world f they are missing but it is "fair" to add something about the error to the log and skip adding a rule for them.
We can easily check if they are explicitly optional and set required false in those cases, but still the result would be a broken apparmor rule (so nothing useful in any way).
Therefore not worth to add logic, instead static "mandatory" and if not a good rc not adding a rule is probably the best.