[libvirt PATCH v2] qemu: tpm: do not update profile name for transient domains
If we do not have a persistent definition, there's no point in looking for it since we cannot store it. Also skip the update if the tpm device(s) in the persistent definition are different. This fixes the crash when starting a transient domain. https://issues.redhat.com/browse/RHEL-69774 Fixes: d79542eec669eb9c449bb8228179e7a87e768017 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_extdevice.c | 12 +++++++++++- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a6f31f9773..21db01a361 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -190,7 +190,17 @@ qemuExtDevicesStart(virQEMUDriver *driver, for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; - virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i]; + virDomainTPMDef *persistentTPMDef = NULL; + + if (persistentDef) { + /* do not try to update the profile in the persistent definition + * if the device does not match */ + if (persistentDef->ntpms == def->ntpms) + persistentTPMDef = persistentDef->tpms[i]; + if (persistentTPMDef->type != tpm->type || + persistentTPMDef->model != tpm->model) + persistentTPMDef = NULL; + } if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && qemuExtTPMStart(driver, vm, tpm, persistentTPMDef, diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index f223dcb9ae..f5e0184e54 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -773,7 +773,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, incomingMigration) < 0) goto error; - if (run_setup && !incomingMigration && + if (run_setup && !incomingMigration && persistentTPMDef && qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef, cfg, saveDef) < 0) goto error; -- 2.47.0
On Tue, Dec 10, 2024 at 15:01:16 +0100, Ján Tomko wrote:
If we do not have a persistent definition, there's no point in looking for it since we cannot store it.
Also skip the update if the tpm device(s) in the persistent definition are different.
This fixes the crash when starting a transient domain.
https://issues.redhat.com/browse/RHEL-69774
Fixes: d79542eec669eb9c449bb8228179e7a87e768017 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_extdevice.c | 12 +++++++++++- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a6f31f9773..21db01a361 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -190,7 +190,17 @@ qemuExtDevicesStart(virQEMUDriver *driver,
for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; - virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i]; + virDomainTPMDef *persistentTPMDef = NULL; + + if (persistentDef) { + /* do not try to update the profile in the persistent definition + * if the device does not match */ + if (persistentDef->ntpms == def->ntpms) + persistentTPMDef = persistentDef->tpms[i];
persistentTPMDef may still be NULL here and the following check will happily dereference it
+ if (persistentTPMDef->type != tpm->type || + persistentTPMDef->model != tpm->model) + persistentTPMDef = NULL; + }
if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
Jirka
On 12/10/24 10:50 AM, Jiri Denemark wrote:
On Tue, Dec 10, 2024 at 15:01:16 +0100, Ján Tomko wrote:
If we do not have a persistent definition, there's no point in looking for it since we cannot store it.
Also skip the update if the tpm device(s) in the persistent definition are different.
This fixes the crash when starting a transient domain.
https://issues.redhat.com/browse/RHEL-69774
Fixes: d79542eec669eb9c449bb8228179e7a87e768017 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_extdevice.c | 12 +++++++++++- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a6f31f9773..21db01a361 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -190,7 +190,17 @@ qemuExtDevicesStart(virQEMUDriver *driver,
for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; - virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i]; + virDomainTPMDef *persistentTPMDef = NULL; + + if (persistentDef) { + /* do not try to update the profile in the persistent definition + * if the device does not match */ + if (persistentDef->ntpms == def->ntpms) + persistentTPMDef = persistentDef->tpms[i];
persistentTPMDef may still be NULL here and the following check will happily dereference it
Hopefully I cannot be NULL from 'persistentTPMDef = persistentDef->tpms[i]' otherwise ntpms would be wrong?! To me the patch looks good...
+ if (persistentTPMDef->type != tpm->type || + persistentTPMDef->model != tpm->model) + persistentTPMDef = NULL; + }
if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
Jirka
On Tue, Dec 10, 2024 at 13:28:44 -0500, Stefan Berger wrote:
On 12/10/24 10:50 AM, Jiri Denemark wrote:
On Tue, Dec 10, 2024 at 15:01:16 +0100, Ján Tomko wrote:
If we do not have a persistent definition, there's no point in looking for it since we cannot store it.
Also skip the update if the tpm device(s) in the persistent definition are different.
This fixes the crash when starting a transient domain.
https://issues.redhat.com/browse/RHEL-69774
Fixes: d79542eec669eb9c449bb8228179e7a87e768017 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_extdevice.c | 12 +++++++++++- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a6f31f9773..21db01a361 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -190,7 +190,17 @@ qemuExtDevicesStart(virQEMUDriver *driver,
for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; - virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i]; + virDomainTPMDef *persistentTPMDef = NULL; + + if (persistentDef) { + /* do not try to update the profile in the persistent definition + * if the device does not match */ + if (persistentDef->ntpms == def->ntpms) + persistentTPMDef = persistentDef->tpms[i];
persistentTPMDef may still be NULL here and the following check will happily dereference it
Hopefully I cannot be NULL from 'persistentTPMDef = persistentDef->tpms[i]' otherwise ntpms would be wrong?!
But persistentTPMDef is only set if persistentDef->ntpms == def->ntpms. So persistentTPMDef will still be NULL here if the number of tpms does not match. Jirka
On 12/10/24 9:01 AM, Ján Tomko wrote:
If we do not have a persistent definition, there's no point in looking for it since we cannot store it.
Also skip the update if the tpm device(s) in the persistent definition are different.
This fixes the crash when starting a transient domain.
https://issues.redhat.com/browse/RHEL-69774
Fixes: d79542eec669eb9c449bb8228179e7a87e768017 Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- src/qemu/qemu_extdevice.c | 12 +++++++++++- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a6f31f9773..21db01a361 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -190,7 +190,17 @@ qemuExtDevicesStart(virQEMUDriver *driver,
for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; - virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i]; + virDomainTPMDef *persistentTPMDef = NULL; + + if (persistentDef) { + /* do not try to update the profile in the persistent definition + * if the device does not match */ + if (persistentDef->ntpms == def->ntpms) + persistentTPMDef = persistentDef->tpms[i]; + if (persistentTPMDef->type != tpm->type || + persistentTPMDef->model != tpm->model) + persistentTPMDef = NULL; + }
if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && qemuExtTPMStart(driver, vm, tpm, persistentTPMDef, diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index f223dcb9ae..f5e0184e54 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -773,7 +773,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, incomingMigration) < 0) goto error;
- if (run_setup && !incomingMigration && + if (run_setup && !incomingMigration && persistentTPMDef && qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef, cfg, saveDef) < 0) goto error;
participants (3)
-
Jiri Denemark -
Ján Tomko -
Stefan Berger