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); } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0bfb6a7fa6..32dc78d777 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1707,9 +1707,8 @@ virSecuritySELinuxRestoreImageLabelSingle(virSecurityManagerPtr mgr, { virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr disk_seclabel; - - if (!src->path || !virStorageSourceIsLocalStorage(src)) - return 0; + g_autofree char *vfioGroupDev = NULL; + const char *path = src->path; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) @@ -1741,9 +1740,16 @@ virSecuritySELinuxRestoreImageLabelSingle(virSecurityManagerPtr mgr, * ownership, because that kills access on the destination host which is * sub-optimal for the guest VM's I/O attempts :-) */ if (migrated) { - int rc = virFileIsSharedFS(src->path); - if (rc < 0) - return -1; + int rc = 1; + + if (virStorageSourceIsLocalStorage(src)) { + if (!src->path) + return 0; + + if ((rc = virFileIsSharedFS(src->path)) < 0) + return -1; + } + if (rc == 1) { VIR_DEBUG("Skipping image label restore on %s because FS is shared", src->path); @@ -1751,7 +1757,22 @@ virSecuritySELinuxRestoreImageLabelSingle(virSecurityManagerPtr mgr, } } - return virSecuritySELinuxRestoreFileLabel(mgr, src->path, true); + /* 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; + + 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. */ + path = vfioGroupDev; + } + + return virSecuritySELinuxRestoreFileLabel(mgr, path, true); } @@ -1798,6 +1819,8 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, char *use_label = NULL; bool remember; bool is_toplevel = parent == src || parent->externalDataStore == src; + g_autofree char *vfioGroupDev = NULL; + const char *path = src->path; int ret; if (!src->path || !virStorageSourceIsLocalStorage(src)) @@ -1850,7 +1873,19 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, use_label = data->content_context; } - ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label, remember); + /* 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; + + if (!(vfioGroupDev = virPCIDeviceAddressGetIOMMUGroupDev(&nvme->pciAddr))) + return -1; + + path = vfioGroupDev; + } + + ret = virSecuritySELinuxSetFilecon(mgr, path, use_label, remember); if (ret == 1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us