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
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 | 23 +++++++++-
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, 97 insertions(+), 53 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a1b1edcbbf51..1f7e587f61b9 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, virDomainDefFree);
+
+ 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, virDomainDefFree);
+
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..9059f9aa3a6c 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,24 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver,
return NULL;
}
+ if (ensureACL) {
+ g_autoptr(virDomainDef) aclDef = NULL;
+
+ /* Avoid parsing the whole domain definition for ACL checks */
+ if (!(aclDef = virDomainDefIDsParseString(dom_xml, driver->xmlopt,
+ VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+ return NULL;
+
+ if (dname) {
+ VIR_FREE(aclDef->name);
+ aclDef->name = g_strdup(dname);
+ }
+
+ if (ensureACL(sconn, aclDef) < 0) {
+ return NULL;
+ }
+ }
+
if (!(def = virDomainDefParseString(dom_xml, driver->xmlopt,
qemuCaps,
VIR_DOMAIN_DEF_PARSE_INACTIVE)))
@@ -4969,6 +4989,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
On 11/11/25 10:26, Martin Kletzander wrote: > 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 | 23 +++++++++- > 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, 97 insertions(+), 53 deletions(-) Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
© 2016 - 2025 Red Hat, Inc.