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@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 21560b2330da57cd2fd9a3596adc146f54fc0f60..7cea7889313bd996730eadb3319ed00c4b0fcf68 100644 (file)
--- 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 a9a1fad5d722935752ab656b579b9336f99b17bb..411aa1b1594cd593126b6160b9a14a3feaefc2e9 100644 (file)
--- 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 0bfb6a7fa6afa9d1c69d78fd7c3bad9b9003d118..32dc78d777c0750b39cbdee463793441348bda60 100644 (file)
--- 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