[libvirt] [RFC PATCH v2 REBASE 07/18] security: dac: Enable labeling of vfio mediated devices

Erik Skultety posted 18 patches 8 years, 11 months ago
There is a newer version of this series
[libvirt] [RFC PATCH v2 REBASE 07/18] security: dac: Enable labeling of vfio mediated devices
Posted by Erik Skultety 8 years, 11 months ago
Label the VFIO IOMMU devices under /dev/vfio/ referenced by the symlinks
in the sysfs (e.g. /sys/class/mdev_bus/<uuid>/iommu_group) which what
qemu actually gets formatted on the command line.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/security/security_dac.c | 57 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index ecce1d3..45bd24e 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -33,6 +33,7 @@
 #include "virfile.h"
 #include "viralloc.h"
 #include "virlog.h"
+#include "virmdev.h"
 #include "virpci.h"
 #include "virusb.h"
 #include "virscsi.h"
@@ -856,6 +857,15 @@ virSecurityDACSetHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
 
 
 static int
+virSecurityDACSetMediatedDevLabel(virMediatedDevicePtr dev ATTRIBUTE_UNUSED,
+                                  const char *file,
+                                  void *opaque)
+{
+    return virSecurityDACSetHostdevLabelHelper(file, opaque);
+}
+
+
+static int
 virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr def,
                               virDomainHostdevDefPtr dev,
@@ -867,7 +877,9 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
     virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
     virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
     virDomainHostdevSubsysSCSIVHostPtr hostsrc = &dev->source.subsys.u.scsi_host;
+    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
     int ret = -1;
+    virMediatedDevicePtr mdev = NULL;
 
     if (!priv->dynamicOwnership)
         return 0;
@@ -964,13 +976,26 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
         break;
     }
 
-    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+        char *vfio_dev = NULL;
+        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr)))
+            goto done;
+
+        if (!(vfio_dev = virMediatedDeviceGetIOMMUGroupDev(mdev)))
+            goto done;
+
+        ret = virSecurityDACSetMediatedDevLabel(mdev, vfio_dev, &cbdata);
+        VIR_FREE(vfio_dev);
+        break;
+    }
+
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
         ret = 0;
         break;
     }
 
  done:
+    virMediatedDeviceFree(mdev);
     return ret;
 }
 
@@ -1018,6 +1043,15 @@ virSecurityDACRestoreHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
     return virSecurityDACRestoreFileLabel(priv, file);
 }
 
+static int
+virSecurityDACRestoreMediatedDevLabel(virMediatedDevicePtr dev ATTRIBUTE_UNUSED,
+                                      const char *file,
+                                      void *opaque)
+{
+    virSecurityManagerPtr mgr = opaque;
+    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+    return virSecurityDACRestoreFileLabel(priv, file);
+}
 
 static int
 virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
@@ -1032,6 +1066,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
     virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
     virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
     virDomainHostdevSubsysSCSIVHostPtr hostsrc = &dev->source.subsys.u.scsi_host;
+    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
     int ret = -1;
 
     secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
@@ -1120,7 +1155,25 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
         break;
     }
 
-    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+        char *vfiodev = NULL;
+        virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr);
+
+        if (!mdev)
+            goto done;
+
+        if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
+            virMediatedDeviceFree(mdev);
+            goto done;
+        }
+
+        ret = virSecurityDACRestoreMediatedDevLabel(mdev, vfiodev, mgr);
+
+        VIR_FREE(vfiodev);
+        virMediatedDeviceFree(mdev);
+        break;
+    }
+
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
         ret = 0;
         break;
-- 
2.10.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 REBASE 07/18] security: dac: Enable labeling of vfio mediated devices
Posted by Pavel Hrdina 8 years, 11 months ago
On Mon, Feb 20, 2017 at 03:28:20PM +0100, Erik Skultety wrote:
> Label the VFIO IOMMU devices under /dev/vfio/ referenced by the symlinks
> in the sysfs (e.g. /sys/class/mdev_bus/<uuid>/iommu_group) which what
> qemu actually gets formatted on the command line.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/security/security_dac.c | 57 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index ecce1d3..45bd24e 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -33,6 +33,7 @@
>  #include "virfile.h"
>  #include "viralloc.h"
>  #include "virlog.h"
> +#include "virmdev.h"
>  #include "virpci.h"
>  #include "virusb.h"
>  #include "virscsi.h"
> @@ -856,6 +857,15 @@ virSecurityDACSetHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
>  
>  
>  static int
> +virSecurityDACSetMediatedDevLabel(virMediatedDevicePtr dev ATTRIBUTE_UNUSED,
> +                                  const char *file,
> +                                  void *opaque)
> +{
> +    return virSecurityDACSetHostdevLabelHelper(file, opaque);
> +}

This wrapper is not required, mediated devices don't have an *Iterate()
function (which is in most cases only yet another wrapper for a simple
function call).

> +
> +
> +static int
>  virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>                                virDomainDefPtr def,
>                                virDomainHostdevDefPtr dev,
> @@ -867,7 +877,9 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>      virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
>      virDomainHostdevSubsysSCSIVHostPtr hostsrc = &dev->source.subsys.u.scsi_host;
> +    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
>      int ret = -1;
> +    virMediatedDevicePtr mdev = NULL;
>  
>      if (!priv->dynamicOwnership)
>          return 0;
> @@ -964,13 +976,26 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>          break;
>      }
>  
> -    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> +        char *vfio_dev = NULL;
> +        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr)))
> +            goto done;
> +
> +        if (!(vfio_dev = virMediatedDeviceGetIOMMUGroupDev(mdev)))
> +            goto done;
> +
> +        ret = virSecurityDACSetMediatedDevLabel(mdev, vfio_dev, &cbdata);

You can use virSecurityDACSetHostdevLabelHelper directly.

> +        VIR_FREE(vfio_dev);
> +        break;
> +    }
> +
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>          ret = 0;
>          break;
>      }
>  
>   done:
> +    virMediatedDeviceFree(mdev);
>      return ret;
>  }
>  
> @@ -1018,6 +1043,15 @@ virSecurityDACRestoreHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
>      return virSecurityDACRestoreFileLabel(priv, file);
>  }
>  
> +static int
> +virSecurityDACRestoreMediatedDevLabel(virMediatedDevicePtr dev ATTRIBUTE_UNUSED,
> +                                      const char *file,
> +                                      void *opaque)
> +{
> +    virSecurityManagerPtr mgr = opaque;
> +    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> +    return virSecurityDACRestoreFileLabel(priv, file);
> +}
>  
>  static int
>  virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
> @@ -1032,6 +1066,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>      virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
>      virDomainHostdevSubsysSCSIVHostPtr hostsrc = &dev->source.subsys.u.scsi_host;
> +    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
>      int ret = -1;
>  
>      secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
> @@ -1120,7 +1155,25 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>          break;
>      }
>  
> -    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> +        char *vfiodev = NULL;
> +        virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr);
> +
> +        if (!mdev)
> +            goto done;
> +
> +        if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> +            virMediatedDeviceFree(mdev);
> +            goto done;
> +        }
> +
> +        ret = virSecurityDACRestoreMediatedDevLabel(mdev, vfiodev, mgr);

Same here, you don't have to use this wrapper, use
virSecurityDACRestoreFileLabel directly.

This applies to security_selinux as well and I think that you should merge
the security_dac and security_selinux patches together and you are missing
security_apparmor patch.

Pavel

> +
> +        VIR_FREE(vfiodev);
> +        virMediatedDeviceFree(mdev);
> +        break;
> +    }
> +
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>          ret = 0;
>          break;
> -- 
> 2.10.2
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list