[PATCH] qemu: backup: Restore security label on backup disk store image on VM termination

Peter Krempa pkrempa at redhat.com
Thu Mar 18 16:46:43 UTC 2021


When the backup job is terminated normally the security label is
restored by the blockjob finishing handler.

If the VM dies or is destroyed that wouldn't happen as the blockjob
handler wouldn't be called.

Restore the security label on disk store where we remember that the job
was running at the point when 'qemuBackupJobTerminate' was called.

Not resetting the security label means that we also leak the xattr
attributes remembering the label which prevents any further use of the
file, which is a problem for block devices.

This also requires that the call to 'qemuBackupJobTerminate' from
'qemuProcessStop' happens only after 'vm->pid' was reset as otherwise
the security subdrivers attempt to enter the process namespace which
fails if the process isn't running any more.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1939082
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_backup.c  | 36 ++++++++++++++++++++++++++----------
 src/qemu/qemu_process.c |  8 ++++----
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index f91d632715..430c11762c 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -28,6 +28,7 @@
 #include "qemu_monitor_json.h"
 #include "qemu_checkpoint.h"
 #include "qemu_command.h"
+#include "qemu_security.h"

 #include "storage_source.h"
 #include "storage_source_conf.h"
@@ -558,25 +559,40 @@ qemuBackupJobTerminate(virDomainObjPtr vm,

 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    g_autoptr(virQEMUDriverConfig) cfg = NULL;
     size_t i;

-    if (!(priv->backup->apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) &&
-        (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PULL ||
-         (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PUSH &&
-          jobstatus != QEMU_DOMAIN_JOB_STATUS_COMPLETED))) {
+    for (i = 0; i < priv->backup->ndisks; i++) {
+        virDomainBackupDiskDefPtr backupdisk = priv->backup->disks + i;

-        g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
+        if (!backupdisk->store)
+            continue;
+
+        /* restore security label on the images in case the blockjob finishing
+         * handler didn't do so, such as when the VM was destroyed */
+        if (backupdisk->state == VIR_DOMAIN_BACKUP_DISK_STATE_RUNNING ||
+            backupdisk->state == VIR_DOMAIN_BACKUP_DISK_STATE_NONE) {
+            if (qemuSecurityRestoreImageLabel(priv->driver, vm, backupdisk->store,
+                                              false) < 0)
+                VIR_WARN("Unable to restore security label on %s",
+                         NULLSTR(backupdisk->store->path));
+        }
+
+        /* delete unneeded images created by libvirt */
+        if (backupdisk->store->type == VIR_STORAGE_TYPE_FILE &&
+            !(priv->backup->apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) &&
+            (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PULL ||
+             (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PUSH &&
+              jobstatus != QEMU_DOMAIN_JOB_STATUS_COMPLETED))) {

-        for (i = 0; i < priv->backup->ndisks; i++) {
-            virDomainBackupDiskDefPtr backupdisk = priv->backup->disks + i;
             uid_t uid;
             gid_t gid;

-            if (!backupdisk->store ||
-                backupdisk->store->type != VIR_STORAGE_TYPE_FILE)
-                continue;
+            if (!cfg)
+                cfg = virQEMUDriverGetConfig(priv->driver);

             qemuDomainGetImageIds(cfg, vm, backupdisk->store, NULL, &uid, &gid);
+
             if (virFileRemove(backupdisk->store->path, uid, gid) < 0)
                 VIR_WARN("failed to remove scratch file '%s'",
                          backupdisk->store->path);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5f31260221..0b79dde2c3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7822,10 +7822,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         virResctrlAllocRemove(vm->def->resctrls[i]->alloc);
     }

-    /* clean up a possible backup job */
-    if (priv->backup)
-        qemuBackupJobTerminate(vm, QEMU_DOMAIN_JOB_STATUS_CANCELED);
-
     qemuProcessRemoveDomainStatus(driver, vm);

     /* Remove VNC and Spice ports from port reservation bitmap, but only if
@@ -7877,6 +7873,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     for (i = 0; i < vm->def->niothreadids; i++)
         vm->def->iothreadids[i]->thread_id = 0;

+    /* clean up a possible backup job */
+    if (priv->backup)
+        qemuBackupJobTerminate(vm, QEMU_DOMAIN_JOB_STATUS_CANCELED);
+
     /* Do this explicitly after vm->pid is reset so that security drivers don't
      * try to enter the domain's namespace which is non-existent by now as qemu
      * is no longer running. */
-- 
2.29.2




More information about the libvir-list mailing list