[libvirt] [PATCH] security: aa-helper: fix static defined vfio MDEVs

Christian Ehrhardt posted 1 patch 5 years, 5 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20181122102736.14853-1-christian.ehrhardt@canonical.com
There is a newer version of this series
src/security/virt-aa-helper.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[libvirt] [PATCH] security: aa-helper: fix static defined vfio MDEVs
Posted by Christian Ehrhardt 5 years, 5 months ago
For vfio MDEVs we need to allow qemu the vfio access in apparmor.

This is extending the older fix 74e86b6b: "Fix apparmor profile
to make vfio pci passthrough work" which was for VFIO PCI
passthrough on static hostdevs to now also cover vfio MDEVs.
It is having the same limitations of the lifecycle at that time
being unable to detect the actual vfio device and therefore
adds a wildcars.

Please also note that hotplug - which in can detect the right
device at runtime - is covered by labeling callbacks in
606afafb: "security: Enable labeling of vfio mediated devices"

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 src/security/virt-aa-helper.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 03cc15c9d3..c7488432d6 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1105,6 +1105,23 @@ get_files(vahControl * ctl)
                 break;
             }
 
+            case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+                virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
+                switch ((virMediatedDeviceModelType) mdevsrc->model) {
+                    case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+                    case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+                    case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
+                        needsVfio = true;
+                        break;
+                    case VIR_MDEV_MODEL_TYPE_LAST:
+                    default:
+                        virReportEnumRangeError(virMediatedDeviceModelType,
+                                                mdevsrc->model);
+                        break;
+                }
+                break;
+            }
+
             case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
                 virPCIDevicePtr pci = virPCIDeviceNew(
                            dev->source.subsys.u.pci.addr.domain,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: aa-helper: fix static defined vfio MDEVs
Posted by Erik Skultety 5 years, 5 months ago
On Thu, Nov 22, 2018 at 11:27:36AM +0100, Christian Ehrhardt wrote:
> For vfio MDEVs we need to allow qemu the vfio access in apparmor.

How about:
"Apparmor needs to be able to grant QEMU access to VFIO MDEV devices."

>
> This is extending the older fix 74e86b6b: "Fix apparmor profile

This extends commit 74e86b6b which only covered PCI hostdevs for VFIO-PCI
assignment.

> to make vfio pci passthrough work" which was for VFIO PCI
> passthrough on static hostdevs to now also cover vfio MDEVs.
> It is having the same limitations of the lifecycle at that time

It has still the same limitations regarding the device lifecycle, IOW we're
unable to predict the actual VFIO device being created, thus we need wildcards.

> being unable to detect the actual vfio device and therefore
> adds a wildcars.
>
> Please also note that hotplug - which in can detect the right
> device at runtime - is covered by labeling callbacks in
> 606afafb: "security: Enable labeling of vfio mediated devices"

Also note that the hotplug case, where apparmor is able to detect the actual
VFIO device during runtime, is already covered by commit 606afafb.

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

Apart from the tiny typographical nit pick I had - you don't even need to agree
to them as I'm not a native speaker myself - I don't see a functional problem
with the patch from libvirt's perspective, so:

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: aa-helper: fix static defined vfio MDEVs
Posted by Christian Ehrhardt 5 years, 5 months ago
On Thu, Nov 22, 2018 at 11:27 AM Christian Ehrhardt <
christian.ehrhardt@canonical.com> wrote:

> For vfio MDEVs we need to allow qemu the vfio access in apparmor.
>
> This is extending the older fix 74e86b6b: "Fix apparmor profile
> to make vfio pci passthrough work" which was for VFIO PCI
> passthrough on static hostdevs to now also cover vfio MDEVs.
> It is having the same limitations of the lifecycle at that time
> being unable to detect the actual vfio device and therefore
> adds a wildcars.
>

obviously wildcards - not afraid of bad traffic, but not worth a V2.
Fixed locally already as well as the first line which had the word "access"
twice.

Waiting for feedback to make a V2 with actual (not just commit words)
changes as needed.

P.S. @Boris as I know you are affected by missing this I you to CC on the
thread as well. Enjoy my typos :-/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: aa-helper: fix static defined vfio MDEVs
Posted by Boris Fiuczynski 5 years, 5 months ago
On 11/22/18 11:32 AM, Christian Ehrhardt wrote:
> On Thu, Nov 22, 2018 at 11:27 AM Christian Ehrhardt <
> christian.ehrhardt@canonical.com> wrote:
> 
>> For vfio MDEVs we need to allow qemu the vfio access in apparmor.
>>
>> This is extending the older fix 74e86b6b: "Fix apparmor profile
>> to make vfio pci passthrough work" which was for VFIO PCI
>> passthrough on static hostdevs to now also cover vfio MDEVs.
>> It is having the same limitations of the lifecycle at that time
>> being unable to detect the actual vfio device and therefore
>> adds a wildcars.
>>
> 
> obviously wildcards - not afraid of bad traffic, but not worth a V2.
> Fixed locally already as well as the first line which had the word "access"
> twice.
> 
> Waiting for feedback to make a V2 with actual (not just commit words)
> changes as needed.
Since the code changes except for vfio-ccw seem to be in line with the 
ppa I tested already

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>


> 
> P.S. @Boris as I know you are affected by missing this I you to CC on the
> thread as well. Enjoy my typos :-/
I sure am! :)

> 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list