[PATCH 7/7] qemu: Check ACLs before parsing the whole domain XML

7 Nov 2025

From: Martin Kletzander <mkletzan@redhat.com>

Utilise the new virDomainDefIDsParseString() for that.

This is one of the more complex ones since there is also a function that
reads relevant metadata from a save image XML.  In order _not_ to extract
the parsing out of the function (and make the function basically trivial
and all callers more complex) add a callback to the function which will
be used to check the ACLs.

Fixes: CVE-2025-12748
Reported-by: Святослав Терешин <s.tereshin@fobos-nt.ru>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_driver.c    | 90 ++++++++++++++++++++-------------------
 src/qemu/qemu_migration.c | 21 ++++++++-
 src/qemu/qemu_migration.h |  4 +-
 src/qemu/qemu_saveimage.c | 25 +++++++++--
 src/qemu/qemu_saveimage.h |  4 +-
 src/qemu/qemu_snapshot.c  |  4 +-
 6 files changed, 95 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a1b1edcbbf51..837935d524bc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1556,11 +1556,17 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
     if (flags & VIR_DOMAIN_START_RESET_NVRAM)
         start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
 
-    if (!(def = virDomainDefParseString(xml, driver->xmlopt,
-                                        NULL, parse_flags)))
-        goto cleanup;
+    /* Avoid parsing the whole domain definition for ACL checks */
+    if (!(def = virDomainDefIDsParseString(xml, driver->xmlopt, parse_flags)))
+        return NULL;
 
     if (virDomainCreateXMLEnsureACL(conn, def) < 0)
+        return NULL;
+
+    g_clear_pointer(&def, virObjectUnref);
+
+    if (!(def = virDomainDefParseString(xml, driver->xmlopt,
+                                        NULL, parse_flags)))
         goto cleanup;
 
     if (!(vm = virDomainObjListAdd(driver->domains, &def,
@@ -5780,7 +5786,7 @@ qemuDomainRestoreInternal(virConnectPtr conn,
     if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
         reset_nvram = true;
 
-    if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data) < 0)
+    if (qemuSaveImageGetMetadata(driver, NULL, path, ensureACL, conn, &def, &data) < 0)
         goto cleanup;
 
     sparse = data->header.format == QEMU_SAVE_FORMAT_SPARSE;
@@ -5793,9 +5799,6 @@ qemuDomainRestoreInternal(virConnectPtr conn,
     if (fd < 0)
         goto cleanup;
 
-    if (ensureACL(conn, def) < 0)
-        goto cleanup;
-
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
         int hookret;
 
@@ -5923,10 +5926,9 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
 
     virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
 
-    if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data) < 0)
-        goto cleanup;
-
-    if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
+    if (qemuSaveImageGetMetadata(driver, NULL, path,
+                                 virDomainSaveImageGetXMLDescEnsureACL,
+                                 conn, &def, &data) < 0)
         goto cleanup;
 
     ret = qemuDomainDefFormatXML(driver, NULL, def, flags);
@@ -5956,7 +5958,9 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
     else if (flags & VIR_DOMAIN_SAVE_PAUSED)
         state = 0;
 
-    if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data) < 0)
+    if (qemuSaveImageGetMetadata(driver, NULL, path,
+                                 virDomainSaveImageDefineXMLEnsureACL,
+                                 conn, &def, &data) < 0)
         goto cleanup;
 
     fd = qemuSaveImageOpen(driver, path, false, false, NULL, true);
@@ -5964,9 +5968,6 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
     if (fd < 0)
         goto cleanup;
 
-    if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
-        goto cleanup;
-
     if (STREQ(data->xml, dxml) &&
         (state < 0 || state == data->header.was_running)) {
         /* no change to the XML */
@@ -6038,7 +6039,8 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
         goto cleanup;
     }
 
-    if (qemuSaveImageGetMetadata(driver, priv->qemuCaps, path, &def, &data) < 0)
+    if (qemuSaveImageGetMetadata(driver, priv->qemuCaps, path,
+                                 NULL, NULL, &def, &data) < 0)
         goto cleanup;
 
     ret = qemuDomainDefFormatXML(driver, priv->qemuCaps, def, flags);
@@ -6102,7 +6104,7 @@ qemuDomainObjRestore(virConnectPtr conn,
     bool sparse = false;
     g_autoptr(qemuMigrationParams) restoreParams = NULL;
 
-    ret = qemuSaveImageGetMetadata(driver, NULL, path, &def, &data);
+    ret = qemuSaveImageGetMetadata(driver, NULL, path, NULL, NULL, &def, &data);
     if (ret < 0) {
         if (qemuSaveImageIsCorrupt(driver, path)) {
             if (unlink(path) < 0) {
@@ -6464,6 +6466,15 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
     if (flags & VIR_DOMAIN_DEFINE_VALIDATE)
         parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
 
+    /* Avoid parsing the whole domain definition for ACL checks */
+    if (!(def = virDomainDefIDsParseString(xml, driver->xmlopt, parse_flags)))
+        return NULL;
+
+    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
+        return NULL;
+
+    g_clear_pointer(&def, virObjectUnref);
+
     if (!(def = virDomainDefParseString(xml, driver->xmlopt,
                                         NULL, parse_flags)))
         return NULL;
@@ -6471,9 +6482,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
     if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
         goto cleanup;
 
-    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
-        goto cleanup;
-
     if (!(vm = virDomainObjListAdd(driver->domains, &def,
                                    driver->xmlopt,
                                    0, &oldDef)))
@@ -10769,10 +10777,9 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn,
         return -1;
     }
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepareTunnelEnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepareTunnelEnsureACL)))
         return -1;
 
     return qemuMigrationDstPrepareTunnel(driver, dconn,
@@ -10822,10 +10829,9 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
         return -1;
     }
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepare2EnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepare2EnsureACL)))
         return -1;
 
     /* Do not use cookies in v2 protocol, since the cookie
@@ -11045,10 +11051,9 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
                                                    QEMU_MIGRATION_DESTINATION)))
         return -1;
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepare3EnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepare3EnsureACL)))
         return -1;
 
     return qemuMigrationDstPrepareDirect(driver, dconn,
@@ -11148,10 +11153,9 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
         return -1;
     }
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepare3ParamsEnsureACL)))
         return -1;
 
     return qemuMigrationDstPrepareDirect(driver, dconn,
@@ -11193,10 +11197,9 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn,
                                                    QEMU_MIGRATION_DESTINATION)))
         return -1;
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepareTunnel3EnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepareTunnel3EnsureACL)))
         return -1;
 
     return qemuMigrationDstPrepareTunnel(driver, dconn,
@@ -11245,10 +11248,9 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
                                                    QEMU_MIGRATION_DESTINATION)))
         return -1;
 
-    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname)))
-        return -1;
-
-    if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0)
+    if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname,
+                                           dconn,
+                                           virDomainMigratePrepareTunnel3ParamsEnsureACL)))
         return -1;
 
     return qemuMigrationDstPrepareTunnel(driver, dconn,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9109c4526db1..dcf9ea444ef9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4030,7 +4030,9 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver,
                            virQEMUCaps *qemuCaps,
                            const char *dom_xml,
                            const char *dname,
-                           char **origname)
+                           char **origname,
+                           virConnectPtr sconn,
+                           int (*ensureACL)(virConnectPtr, virDomainDef *))
 {
     virDomainDef *def;
     char *name = NULL;
@@ -4041,6 +4043,22 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver,
         return NULL;
     }
 
+    /* Avoid parsing the whole domain definition for ACL checks */
+    if (!(def = virDomainDefIDsParseString(dom_xml, driver->xmlopt,
+                                           VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+        goto cleanup;
+
+    if (dname) {
+        VIR_FREE(def->name);
+        def->name = g_strdup(dname);
+    }
+
+    if (ensureACL && ensureACL(sconn, def) < 0) {
+        g_clear_pointer(&def, virObjectUnref);
+        goto cleanup;
+    }
+    g_clear_pointer(&def, virObjectUnref);
+
     if (!(def = virDomainDefParseString(dom_xml, driver->xmlopt,
                                         qemuCaps,
                                         VIR_DOMAIN_DEF_PARSE_INACTIVE)))
@@ -4969,6 +4987,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver,
             if (!(persistDef = qemuMigrationAnyPrepareDef(driver,
                                                           priv->qemuCaps,
                                                           persist_xml,
+                                                          NULL, NULL,
                                                           NULL, NULL)))
                 goto error;
         } else {
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 36865040dffc..50910ecb1f92 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -134,7 +134,9 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver,
                            virQEMUCaps *qemuCaps,
                            const char *dom_xml,
                            const char *dname,
-                           char **origname);
+                           char **origname,
+                           virConnectPtr sconn,
+                           int (*ensureACL)(virConnectPtr, virDomainDef *));
 
 int
 qemuMigrationDstPrepareTunnel(virQEMUDriver *driver,
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index aa030798ce19..145a0f483283 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -614,16 +614,21 @@ qemuSaveImageIsCorrupt(virQEMUDriver *driver, const char *path)
  * @driver: qemu driver data
  * @qemuCaps: pointer to qemuCaps if the domain is running or NULL
  * @path: path of the save image
+ * @ensureACL: ACL callback to check against the definition or NULL
+ * @conn: parameter for the @ensureACL callback
  * @ret_def: returns domain definition created from the XML stored in the image
  * @ret_data: returns structure filled with data from the image header
  *
- * Open the save image file, read libvirt's save image metadata, and populate
- * the @ret_def and @ret_data structures. Returns 0 on success and -1 on failure.
+ * Open the save image file, read libvirt's save image metadata, optionally
+ * check ACLs before parsing the whole domain definition and populate the
+ * @ret_def and @ret_data structures. Returns 0 on success and -1 on failure.
  */
 int
 qemuSaveImageGetMetadata(virQEMUDriver *driver,
                          virQEMUCaps *qemuCaps,
                          const char *path,
+                         int (*ensureACL)(virConnectPtr, virDomainDef *),
+                         virConnectPtr conn,
                          virDomainDef **ret_def,
                          virQEMUSaveData **ret_data)
 {
@@ -631,6 +636,8 @@ qemuSaveImageGetMetadata(virQEMUDriver *driver,
     VIR_AUTOCLOSE fd = -1;
     virQEMUSaveData *data;
     g_autoptr(virDomainDef) def = NULL;
+    unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+                               VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
     int rc;
 
     if ((fd = qemuDomainOpenFile(cfg, NULL, path, O_RDONLY, NULL)) < 0)
@@ -640,10 +647,20 @@ qemuSaveImageGetMetadata(virQEMUDriver *driver,
         return rc;
 
     data = *ret_data;
+
+    if (ensureACL) {
+        /* Parse only the IDs for ACL checks */
+        g_autoptr(virDomainDef) aclDef = virDomainDefIDsParseString(data->xml,
+                                                                    driver->xmlopt,
+                                                                    parse_flags);
+
+        if (!aclDef || ensureACL(conn, aclDef) < 0)
+            return -1;
+    }
+
     /* Create a domain from this XML */
     if (!(def = virDomainDefParseString(data->xml, driver->xmlopt, qemuCaps,
-                                        VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                        VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+                                        parse_flags)))
         return -1;
 
     *ret_def = g_steal_pointer(&def);
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index 89c694138505..15b73eb39575 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -98,9 +98,11 @@ int
 qemuSaveImageGetMetadata(virQEMUDriver *driver,
                          virQEMUCaps *qemuCaps,
                          const char *path,
+                         int (*ensureACL)(virConnectPtr, virDomainDef *),
+                         virConnectPtr conn,
                          virDomainDef **ret_def,
                          virQEMUSaveData **ret_data)
-    ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+    ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7);
 
 int
 qemuSaveImageOpen(virQEMUDriver *driver,
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d4994dd54ed7..5aa7d1b3a79d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2486,8 +2486,8 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
         g_autoptr(virDomainDef) savedef = NULL;
 
         memdata->path = snapdef->memorysnapshotfile;
-        if (qemuSaveImageGetMetadata(driver, NULL, memdata->path, &savedef,
-                                     &memdata->data) < 0)
+        if (qemuSaveImageGetMetadata(driver, NULL, memdata->path, NULL, NULL,
+                                     &savedef, &memdata->data) < 0)
             return -1;
 
         memdata->fd = qemuSaveImageOpen(driver, memdata->path,
-- 
2.51.2

    