[libvirt] [PATCH 0/2] qemu: migration: Fix crash when adding TLS objects
Thankfully no other caller of qemuMonitorAddObject uses double pointers so the checks are correct. Peter Krempa (2): qemu: hotplug: Do not try to add secret object for TLS if it does not exist qemu: monitor: Make qemuMonitorAddObject more robust against programming errors src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_monitor.c | 22 ++++++++++++++++------ src/qemu/qemu_monitor.h | 3 ++- 3 files changed, 19 insertions(+), 8 deletions(-) -- 2.16.2
The check whether the object holding secret for decryption of the TLS environment was wrong and would always attempt to add the object. This lead to a crash due to recent refactors. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598015 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 23f6d1daba..fe99c0aca2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1338,7 +1338,7 @@ qemuDomainAddTLSObjects(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - if (secProps && + if (secProps && *secProps && qemuMonitorAddObject(priv->mon, secProps, &secAlias) < 0) goto error; -- 2.16.2
On Wed, Jul 04, 2018 at 04:40:55PM +0200, Peter Krempa wrote:
The check whether the object holding secret for decryption of the TLS environment was wrong and would always attempt to add the object. This lead to a crash due to recent refactors.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598015
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Document and check that @props contains a pointer to a json object and check that both necessary fields are present. Also marm @props as NONNULL. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 22 ++++++++++++++++------ src/qemu/qemu_monitor.h | 3 ++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6ed475ede0..c1afe21cc1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3073,8 +3073,9 @@ qemuMonitorCreateObjectProps(virJSONValuePtr *propsret, /** * qemuMonitorAddObject: * @mon: Pointer to monitor object - * @props: Optional arguments for the given type. The object is consumed and - * the pointer is cleared. + * @props: Pointer to a JSON object holding configuration of the object to add. + * The object must be non-null and contain at least the "qom-type" and + * "id" field. The object is consumed and the pointer is cleared. * @alias: If not NULL, returns the alias of the added object if it was added * successfully to qemu. Caller should free the returned pointer. * @@ -3085,18 +3086,27 @@ qemuMonitorAddObject(qemuMonitorPtr mon, virJSONValuePtr *props, char **alias) { - const char *type = virJSONValueObjectGetString(*props, "qom-type"); - const char *id = virJSONValueObjectGetString(*props, "id"); + const char *type = NULL; + const char *id = NULL; char *tmp = NULL; int ret = -1; + if (!*props) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("object props can't be NULL")); + goto cleanup; + } + + type = virJSONValueObjectGetString(*props, "qom-type"); + id = virJSONValueObjectGetString(*props, "id"); + VIR_DEBUG("type=%s id=%s", NULLSTR(type), NULLSTR(id)); QEMU_CHECK_MONITOR_GOTO(mon, cleanup); - if (!id) { + if (!id || !type) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing alias for qemu object '%s'"), NULLSTR(type)); + _("missing alias or qom-type for qemu object '%s'"), + NULLSTR(type)); goto cleanup; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..e8adda8aa0 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -812,7 +812,8 @@ int qemuMonitorCreateObjectProps(virJSONValuePtr *propsret, int qemuMonitorAddObject(qemuMonitorPtr mon, virJSONValuePtr *props, - char **alias); + char **alias) + ATTRIBUTE_NONNULL(1); int qemuMonitorDelObject(qemuMonitorPtr mon, const char *objalias); -- 2.16.2
Document and check that @props contains a pointer to a json object and check that both necessary fields are present. Also mark @props as NONNULL. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Diff: - added the not commited fix to add "%s" to the error message so as a syntax-check fix - typo fix in commit message src/qemu/qemu_monitor.c | 23 +++++++++++++++++------ src/qemu/qemu_monitor.h | 3 ++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6ed475ede0..ae5b23b9fc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3073,8 +3073,9 @@ qemuMonitorCreateObjectProps(virJSONValuePtr *propsret, /** * qemuMonitorAddObject: * @mon: Pointer to monitor object - * @props: Optional arguments for the given type. The object is consumed and - * the pointer is cleared. + * @props: Pointer to a JSON object holding configuration of the object to add. + * The object must be non-null and contain at least the "qom-type" and + * "id" field. The object is consumed and the pointer is cleared. * @alias: If not NULL, returns the alias of the added object if it was added * successfully to qemu. Caller should free the returned pointer. * @@ -3085,18 +3086,28 @@ qemuMonitorAddObject(qemuMonitorPtr mon, virJSONValuePtr *props, char **alias) { - const char *type = virJSONValueObjectGetString(*props, "qom-type"); - const char *id = virJSONValueObjectGetString(*props, "id"); + const char *type = NULL; + const char *id = NULL; char *tmp = NULL; int ret = -1; + if (!*props) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("object props can't be NULL")); + goto cleanup; + } + + type = virJSONValueObjectGetString(*props, "qom-type"); + id = virJSONValueObjectGetString(*props, "id"); + VIR_DEBUG("type=%s id=%s", NULLSTR(type), NULLSTR(id)); QEMU_CHECK_MONITOR_GOTO(mon, cleanup); - if (!id) { + if (!id || !type) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing alias for qemu object '%s'"), NULLSTR(type)); + _("missing alias or qom-type for qemu object '%s'"), + NULLSTR(type)); goto cleanup; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..e8adda8aa0 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -812,7 +812,8 @@ int qemuMonitorCreateObjectProps(virJSONValuePtr *propsret, int qemuMonitorAddObject(qemuMonitorPtr mon, virJSONValuePtr *props, - char **alias); + char **alias) + ATTRIBUTE_NONNULL(1); int qemuMonitorDelObject(qemuMonitorPtr mon, const char *objalias); -- 2.16.2
On Wed, Jul 04, 2018 at 04:51:04PM +0200, Peter Krempa wrote:
Document and check that @props contains a pointer to a json object and check that both necessary fields are present. Also mark @props as NONNULL.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Diff: - added the not commited fix to add "%s" to the error message so as a syntax-check fix - typo fix in commit message
src/qemu/qemu_monitor.c | 23 +++++++++++++++++------ src/qemu/qemu_monitor.h | 3 ++- 2 files changed, 19 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa