<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 20, 2017 at 12:58 PM, Christian Ehrhardt <span dir="ltr"><<a href="mailto:christian.ehrhardt@canonical.com" target="_blank">christian.ehrhardt@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Dec 19, 2017 at 5:26 PM, Jamie Strandboge <<a href="mailto:jamie@canonical.com">jamie@canonical.com</a>> wrote:<br>
> On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:<br>
>> vfio devices are generated on the fly, but the generic base is<br>
>> missing.<br>
>><br>
>> The base vfio has not much functionality but to provide a custom<br>
>> container by opening this path.<br>
>> See <a href="https://www.kernel.org/doc/Documentation/vfio.txt" rel="noreferrer" target="_blank">https://www.kernel.org/doc/<wbr>Documentation/vfio.txt</a> for more.<br>
>><br>
>> Current access by qemu is "wr":<br>
>> [ 2652.756712] audit: type=1400 audit(1491303691.719:25):<br>
>> apparmor="DENIED" operation="open"<br>
>> profile="libvirt-17a61b87-<wbr>5132-497c-b928-421ac2ee0c8a"<br>
>> name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"<br>
>> requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0<br>
>><br>
>> Bug-Ubuntu: <a href="https://bugs.launchpad.net/bugs/1678322" rel="noreferrer" target="_blank">https://bugs.launchpad.net/<wbr>bugs/1678322</a><br>
>><br>
>> Signed-off-by: Christian Ehrhardt <<a href="mailto:christian.ehrhardt@canonical.com">christian.ehrhardt@canonical.<wbr>com</a>><br>
>> Signed-off-by: Stefan Bader <<a href="mailto:stefan.bader@canonical.com">stefan.bader@canonical.com</a>><br>
>> ---<br>
>>  examples/apparmor/libvirt-qemu | 3 +++<br>
>>  1 file changed, 3 insertions(+)<br>
>><br>
>> diff --git a/examples/apparmor/libvirt-<wbr>qemu<br>
>> b/examples/apparmor/libvirt-<wbr>qemu<br>
>> index 5d811f9..eb4d58c 100644<br>
>> --- a/examples/apparmor/libvirt-<wbr>qemu<br>
>> +++ b/examples/apparmor/libvirt-<wbr>qemu<br>
>> @@ -212,3 +212,6 @@<br>
>>    # silence refusals to open lttng files (see LP: #1432644)<br>
>>    deny /dev/shm/lttng-ust-wait-* r,<br>
>>    deny /run/shm/lttng-ust-wait-* r,<br>
>> +<br>
>> +  # for vfio (LP: #1678322)<br>
>> +  /dev/vfio/vfio rw,<br>
><br>
> Why not just also add this rule iff there is a vfio-specific device<br>
> rule? Ie, just add this along with the vfio device rule in virt-aa-<br>
> helper instead of giving all VMs access when it isn't needed.<br>
<br>
</div></div>Good suggestion, but I realized that - other than the per-device rules<br>
that I knew - we already do so for the base dev as well [1].<br>
Thanks for making me aware.<br>
<br>
One thing that puzzles me still is that I hit this in 2017 on a rather<br>
new libvirt.<br>
Maybe on that case the detection was broken and thereby the rule<br>
missing - I'll go back and take a look again and might come up with a<br>
fix for that logic then.<br>
<br>
TL;DR - this is just old garbage, I'm dropping this on my end - yay cleanup !<br></blockquote><div><br></div><div>I finally found why we still need the change, giving a TL;DR here and then submitting a refreshed patch.</div><div><br></div><div>If a guest description has no hostdev's defined, then in the current virt-aa-helper code it will not add /dev/vfio/vfio nor any per vfio device rules.</div><div>That works for guests that have</div><div>a) no hostdevs</div><div>b) hostdevs defined in the guest xml</div><div>It even works for</div><div>c) <span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">hostdevs defined in the guest xml, later more hostdevs hotplugged</span></div><div><br></div><div>But there is another case</div><div>d) guest has no hostdev in guest xml, later hostdevs get hotplugged</div><div><br></div><div>In case (d) virt-aa-helper did not add the rule since at the time there was no need.</div><div>And the way libvirt/qemu currently play together on vfio hotplug leads to the deny of /dev/vfio/vfio as any labeling hooks or similar I have seen are too late.</div><div><br></div><div>If we allow the rather safe /dev/vfio/vfio which is not giving you any device access anyway (see kernel doc) we are good.</div><div>In that case qemu will be allowed to open vfio to get its own vfio space.</div><div>Later libvirt will catch all that is needed and a labeling call lets virt-aa-helper add the right vfio device.</div><div><br></div><div>So we end up with the proposed safe allowance here in the global profile.</div>And the actual device only being in the per guest profile.<div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">  /dev/vfio/93</span><br></span><br></div><div>re-submitting the patch in a few minutes ...</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[1]: <a href="https://libvirt.org/git/?p=libvirt.git;a=commit;h=74e86b6b2521881808bb93290bcebcb469ab7820" rel="noreferrer" target="_blank">https://libvirt.org/git/?p=<wbr>libvirt.git;a=commit;h=<wbr>74e86b6b2521881808bb93290bcebc<wbr>b469ab7820</a><br>
<div class="HOEnZb"><div class="h5"><br>
> --<br>
> Jamie Strandboge             | <a href="http://www.canonical.com" rel="noreferrer" target="_blank">http://www.canonical.com</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Christian Ehrhardt</span><div style="color:rgb(136,136,136);font-size:12.8px">Software Engineer, Ubuntu Server</div><div style="color:rgb(136,136,136);font-size:12.8px">Canonical Ltd</div></div></div></div></div>
</div></div>