[libvirt] [PATCH v4 06/14] security: Enable labeling of vfio mediated devices
Laine Stump
laine at laine.org
Sun Mar 26 18:25:02 UTC 2017
On 03/22/2017 11:27 AM, 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.
The sentence above is confused (i.e. I don't understand it), but I won't
know how to unconfuse it until I've gone through the patch.
> This patch updates all
> of our security drivers.
>
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
> src/security/security_apparmor.c | 21 +++++++++++++++++-
> src/security/security_dac.c | 45 ++++++++++++++++++++++++++++++++++++--
> src/security/security_selinux.c | 47 ++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 108 insertions(+), 5 deletions(-)
>
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index f5b72e1c2d..fc55815261 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -51,6 +51,7 @@
> #include "virlog.h"
> #include "virstring.h"
> #include "virscsi.h"
> +#include "virmdev.h"
>
> #define VIR_FROM_THIS VIR_FROM_SECURITY
>
> @@ -813,6 +814,7 @@ AppArmorSetSecurityHostdevLabel(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;
>
> if (!secdef || !secdef->relabel)
> return 0;
> @@ -901,8 +903,25 @@ AppArmorSetSecurityHostdevLabel(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,
> + mdevsrc->model);
> +
> + if (!mdev)
> + goto done;
> +
> + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> + virMediatedDeviceFree(mdev);
> + goto done;
> + }
Going through the various patches and seeing this (or similar) sequences
so often makes me think it might be cleaner to have APIs that take a
uuidstr and model instead (or maybe define
virDomainHostdevSubsysMediatedDevPtr in util instead of conf, then pass
the mdevsrc directly - that would make it continue to work if/once we
add different ways to specify the device.
But things currently work exactly the same way for PCI devices, so no
sense rewriting just for that. These are all internal APIs, so we can
tweak them to our hearts' content in the future.
> +
> + ret = AppArmorSetSecurityHostdevLabelHelper(vfiodev, ptr);
> +
> + VIR_FREE(vfiodev);
> + virMediatedDeviceFree(mdev);
> break;
> + }
>
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 4e968f29c0..922e484942 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"
> @@ -867,6 +868,7 @@ 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;
>
> if (!priv->dynamicOwnership)
> @@ -964,7 +966,26 @@ virSecurityDACSetHostdevLabel(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,
> + mdevsrc->model);
> +
> + if (!mdev)
> + goto done;
> +
> + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> + virMediatedDeviceFree(mdev);
> + goto done;
> + }
(see what I mean?)
> +
> + ret = virSecurityDACSetHostdevLabelHelper(vfiodev, &cbdata);
> +
> + VIR_FREE(vfiodev);
> + virMediatedDeviceFree(mdev);
> + break;
> + }
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> @@ -1032,6 +1053,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 +1142,26 @@ 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,
> + mdevsrc->model);
> +
> + if (!mdev)
> + goto done;
> +
> + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> + virMediatedDeviceFree(mdev);
> + goto done;
> + }
> +
> + ret = virSecurityDACRestoreFileLabel(virSecurityManagerGetPrivateData(mgr),
> + vfiodev);
> + VIR_FREE(vfiodev);
> + virMediatedDeviceFree(mdev);
> + break;
> + }
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 7b3276dc34..df7c96833e 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -36,6 +36,7 @@
> #include "virerror.h"
> #include "viralloc.h"
> #include "virlog.h"
> +#include "virmdev.h"
> #include "virpci.h"
> #include "virusb.h"
> #include "virscsi.h"
> @@ -1741,6 +1742,7 @@ virSecuritySELinuxSetHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
> return virSecuritySELinuxSetHostdevLabelHelper(file, opaque);
> }
>
> +
> static int
> virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> @@ -1752,6 +1754,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(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;
> virSecuritySELinuxCallbackData data = {.mgr = mgr, .def = def};
>
> int ret = -1;
> @@ -1838,7 +1841,26 @@ virSecuritySELinuxSetHostdevSubsysLabel(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,
> + mdevsrc->model);
> +
> + if (!mdev)
> + goto done;
> +
> + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> + virMediatedDeviceFree(mdev);
> + goto done;
> + }
> +
> + ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, &data);
> +
> + VIR_FREE(vfiodev);
> + virMediatedDeviceFree(mdev);
> + break;
> + }
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> @@ -1973,6 +1995,7 @@ virSecuritySELinuxRestoreHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
> return virSecuritySELinuxRestoreFileLabel(mgr, file);
> }
>
> +
> static int
> virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
> virDomainHostdevDefPtr dev,
> @@ -1983,6 +2006,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(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;
>
> /* Like virSecuritySELinuxRestoreImageLabelInt() for a networked
> @@ -2066,7 +2090,26 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(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,
> + mdevsrc->model);
> +
> + if (!mdev)
> + goto done;
> +
> + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> + virMediatedDeviceFree(mdev);
> + goto done;
> + }
> +
> + ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev);
> +
> + VIR_FREE(vfiodev);
> + virMediatedDeviceFree(mdev);
> + break;
> + }
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
>
ACK.
More information about the libvir-list
mailing list