storage: qemu: Fix security labelling of new image chain elements

When creating a disk image snapshot the libvirt code would blindly copy
the parents label to the newly created image. This runs into problems
when you start a VM from an image hosted on NFS (or other storage system
that doesn't support selinux labels) and the snapshot destination is on
a storage system that does support selinux labels. Libvirt's code in
that case generates a different security label for the image hosted on
NFS. This label is valid only for NFS images and doesn't allow access in
case of a locally stored image.

To fix this issue libvirt needs to refrain from copying security
information in cases where the default domain seclabel is a better
choice.

This patch repurposes the now unused @force argument of
virStorageSourceInitChainElement to denote whether a copy of the
security labelling stuff should be attempted or not. This allows to
fine-control the copy operation for cases where we need to keep the
label of the old disk vs. the cases where we need to keep the label
unset to use the default domain imagelabel.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1151718
(cherry picked from commit 7e130e8b3505ce0f821081dffde8c13a7ff921b3)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d8d898976659bbaeda6b8919351cc6088e968219..04e93feb106f18499ecf04fa51f3b1a903645357 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15579,7 +15579,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                           unsigned long long bandwidth,
                           unsigned int granularity,
                           unsigned long long buf_size,
-                          unsigned int flags)
+                          unsigned int flags,
+                          bool keepParentLabel)
 {
     virQEMUDriverPtr driver = conn->privateData;
     qemuDomainObjPrivatePtr priv;
@@ -15711,7 +15712,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
     if (mirror->format > 0)
         format = virStorageFileFormatTypeToString(mirror->format);
 
-    if (virStorageSourceInitChainElement(mirror, disk->src, false) < 0)
+    if (virStorageSourceInitChainElement(mirror, disk->src,
+                                         keepParentLabel) < 0)
         goto endjob;
 
     if (qemuDomainPrepareDiskChainElement(driver, vm, mirror,
@@ -15823,7 +15825,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
     flags &= (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
               VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT);
     ret = qemuDomainBlockCopyCommon(vm, dom->conn, path, dest,
-                                    bandwidth, 0, 0, flags);
+                                    bandwidth, 0, 0, flags, true);
     vm = NULL;
     dest = NULL;
 
@@ -15896,8 +15898,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml,
                                              VIR_DOMAIN_XML_INACTIVE)))
         goto cleanup;
 
-    ret = qemuDomainBlockCopyCommon(vm, dom->conn, disk, dest,
-                                    bandwidth, granularity, buf_size, flags);
+    ret = qemuDomainBlockCopyCommon(vm, dom->conn, disk, dest, bandwidth,
+                                    granularity, buf_size, flags, false);
     vm = NULL;
 
  cleanup:
@@ -16072,7 +16074,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
             goto endjob;
         if (virStorageSourceInitChainElement(mirror,
                                              disk->src,
-                                             false) < 0)
+                                             true) < 0)
             goto endjob;
     }
 

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6c635c2cd5029ac48616a66a3d98c8c60839fc58..8185d1294a7cafd57f4ad80e51b760ce22916c77 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1064,7 +1064,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                         copy = virStorageSourceCopy(disk->mirror, false);
                         if (virStorageSourceInitChainElement(copy,
                                                              persistDisk->src,
-                                                             false) < 0) {
+                                                             true) < 0) {
                             VIR_WARN("Unable to update persistent definition "
                                      "on vm %s after block job",
                                      vm->def->name);

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4a3b7ca8e1af43f12c4432c982e59ecf3b8aa4a0..064b4f0d010b8bd29fab067bf20cadfa2215ae57 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1901,29 +1901,26 @@ virStorageSourceCopy(const virStorageSource *src,
  * virStorageSourceInitChainElement:
  * @newelem: New backing chain element disk source
  * @old: Existing top level disk source
- * @force: Force-copy the information
+ * @transferLabels: Transfer security lables.
  *
  * Transfers relevant information from the existing disk source to the new
  * backing chain element if they weren't supplied so that labelling info
  * and possibly other stuff is correct.
  *
- * If @force is true, user-supplied information for the new backing store
- * element is overwritten from @old instead of keeping it.
+ * If @transferLabels is true, security labels from the existing disk are copied
+ * to the new disk. Otherwise the default domain imagelabel label will be used.
  *
  * Returns 0 on success, -1 on error.
  */
 int
 virStorageSourceInitChainElement(virStorageSourcePtr newelem,
                                  virStorageSourcePtr old,
-                                 bool force)
+                                 bool transferLabels)
 {
     int ret = -1;
 
-    if (force) {
-        virStorageSourceSeclabelsClear(newelem);
-    }
-
-    if (!newelem->seclabels &&
+    if (transferLabels &&
+        !newelem->seclabels &&
         virStorageSourceSeclabelsCopy(newelem, old) < 0)
         goto cleanup;
 
@@ -2370,7 +2367,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent)
         }
 
         /* copy parent's labelling and other top level stuff */
-        if (virStorageSourceInitChainElement(ret, parent, false) < 0)
+        if (virStorageSourceInitChainElement(ret, parent, true) < 0)
             goto error;
     }