Re: [libvirt] [PATCH v3 23/30] virSecuritySELinuxRestoreImageLabelInt: Don't skip non-local storage

This function is currently not called for any type of storage source that is not considered 'local' (as defined by virStorageSourceIsLocalStorage()). Well, NVMe disks are not 'local' from that point of view and therefore we will need to call this function more frequently. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/security_apparmor.c | 25 ++++++++++++---- src/security/security_dac.c | 30 +++++++++++++++++++ src/security/security_selinux.c | 51 +++++++++++++++++++++++++++----- 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 21560b2330..7cea788931 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -779,9 +779,8 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { virSecurityLabelDefPtr secdef; - - if (!src->path || !virStorageSourceIsLocalStorage(src)) - return 0; + g_autofree char *vfioGroupDev = NULL; + const char *path; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); if (!secdef || !secdef->relabel) @@ -790,15 +789,29 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!secdef->imagelabel) return 0; + if (src->type == VIR_STORAGE_TYPE_NVME) { + const virStorageSourceNVMeDef *nvme = src->nvme; + + if (!(vfioGroupDev = virPCIDeviceAddressGetIOMMUGroupDev(&nvme->pciAddr))) + return -1; + + path = vfioGroupDev; + } else { + if (!src->path || !virStorageSourceIsLocalStorage(src)) + return 0; + + path = src->path; + } + /* if the device doesn't exist, error out */ - if (!virFileExists(src->path)) { + if (!virFileExists(path)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("\'%s\' does not exist"), - src->path); + path); return -1; } - return reload_profile(mgr, def, src->path, true); + return reload_profile(mgr, def, path, true); } static int diff --git a/src/security/security_dac.c b/src/security/security_dac.c index a9a1fad5d7..411aa1b159 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -911,6 +911,19 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, return -1; } + /* This is not very clean. But so far we don't have NVMe + * storage pool backend so that its chownCallback would be + * called. And this place looks least offensive. */ + if (src->type == VIR_STORAGE_TYPE_NVME) { + const virStorageSourceNVMeDef *nvme = src->nvme; + g_autofree char *vfioGroupDev = NULL; + + if (!(vfioGroupDev = virPCIDeviceAddressGetIOMMUGroupDev(&nvme->pciAddr))) + return -1; + + return virSecurityDACSetOwnership(mgr, NULL, vfioGroupDev, user, group, false); + } + /* We can't do restore on shared resources safely. Not even * with refcounting implemented in XATTRs because if there * was a domain running with the feature turned off the @@ -1017,6 +1030,23 @@ virSecurityDACRestoreImageLabelSingle(virSecurityManagerPtr mgr, } } + /* This is not very clean. But so far we don't have NVMe + * storage pool backend so that its chownCallback would be + * called. And this place looks least offensive. */ + if (src->type == VIR_STORAGE_TYPE_NVME) { + const virStorageSourceNVMeDef *nvme = src->nvme; + g_autofree char *vfioGroupDev = NULL; + + if (!(vfioGroupDev = virPCIDeviceAddressGetIOMMUGroupDev(&nvme->pciAddr))) + return -1; + + /* Ideally, we would check if there is not another PCI + * device within domain def that is in the same IOMMU + * group. But we're not doing that for hostdevs yet. */ + + return virSecurityDACRestoreFileLabelInternal(mgr, NULL, vfioGroupDev, false); + } + return virSecurityDACRestoreFileLabelInternal(mgr, src, NULL, true); }