Re: [Qemu-devel] [PATCH v4 2/3] qapi: fix crash when a parameter is missing
Hi Markus
- Original Message -
> Markus Armbruster writes:
>
> > Marc-André Lureau writes:
> >
> >> Calling:
> >>
> >> { "execute": "qom-set",
> >> "arguments": { "path": "/machine", "property": "rtc-time" } }
> >>
> >> Will crash with:
> >>
> >> qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
> >> failed
> >
> > This is actually a recent regression. Let's add "Broken in commit
> > 5c678ee." Can do on commit.
> >
> >> Clear the obj and return an error.
> >>
> >> The patch also fixes a similar potential crash in qmp_input_type_null()
> >> by checking qmp_input_get_object() returned a valid qobj.
> >>
> >> Signed-off-by: Marc-André Lureau
> >> Reviewed-by: Eric Blake
>
> I'd like to rephrase like this, if it's all right with you:
>
> qapi: Fix crash when 'any' or 'null' parameter is missing
>
> Unlike the other visit methods, visit_type_any() and visit_type_null()
> neglect to check whether qmp_input_get_object() succeeded. They crash
> when it fails. Reproducer:
>
> { "execute": "qom-set",
> "arguments": { "path": "/machine", "property": "rtc-time" } }
>
> Will crash with:
>
> qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
> failed
>
> Broken in commit 5c678ee. Fix by adding the missing error checks.
>
> Also:
> Reviewed-by: Markus Armbruster
Looks good to me, thanks
>
Re: [Qemu-devel] [PATCH v4 2/3] qapi: fix crash when a parameter is missing
Markus Armbruster writes:
> Marc-André Lureau writes:
>
>> Calling:
>>
>> { "execute": "qom-set",
>> "arguments": { "path": "/machine", "property": "rtc-time" } }
>>
>> Will crash with:
>>
>> qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
>> failed
>
> This is actually a recent regression. Let's add "Broken in commit
> 5c678ee." Can do on commit.
>
>> Clear the obj and return an error.
>>
>> The patch also fixes a similar potential crash in qmp_input_type_null()
>> by checking qmp_input_get_object() returned a valid qobj.
>>
>> Signed-off-by: Marc-André Lureau
>> Reviewed-by: Eric Blake
I'd like to rephrase like this, if it's all right with you:
qapi: Fix crash when 'any' or 'null' parameter is missing
Unlike the other visit methods, visit_type_any() and visit_type_null()
neglect to check whether qmp_input_get_object() succeeded. They crash
when it fails. Reproducer:
{ "execute": "qom-set",
"arguments": { "path": "/machine", "property": "rtc-time" } }
Will crash with:
qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
failed
Broken in commit 5c678ee. Fix by adding the missing error checks.
Also:
Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v4 2/3] qapi: fix crash when a parameter is missing
Marc-André Lureau writes:
> Calling:
>
> { "execute": "qom-set",
> "arguments": { "path": "/machine", "property": "rtc-time" } }
>
> Will crash with:
>
> qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
> failed
This is actually a recent regression. Let's add "Broken in commit
5c678ee." Can do on commit.
> Clear the obj and return an error.
>
> The patch also fixes a similar potential crash in qmp_input_type_null()
> by checking qmp_input_get_object() returned a valid qobj.
>
> Signed-off-by: Marc-André Lureau
> Reviewed-by: Eric Blake
> ---
> qapi/qmp-input-visitor.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 64dd392..fc91e74 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -338,6 +338,12 @@ static void qmp_input_type_any(Visitor *v, const char
> *name, QObject **obj,
> QmpInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qmp_input_get_object(qiv, name, true);
>
> +if (!qobj) {
> +error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
> +*obj = NULL;
> +return;
> +}
> +
> qobject_incref(qobj);
> *obj = qobj;
> }
> @@ -347,6 +353,11 @@ static void qmp_input_type_null(Visitor *v, const char
> *name, Error **errp)
> QmpInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qmp_input_get_object(qiv, name, true);
>
> +if (!qobj) {
> +error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
> +return;
> +}
> +
> if (qobject_type(qobj) != QTYPE_QNULL) {
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> "null");
[Qemu-devel] [PATCH v4 2/3] qapi: fix crash when a parameter is missing
Calling:
{ "execute": "qom-set",
"arguments": { "path": "/machine", "property": "rtc-time" } }
Will crash with:
qapi/qapi-visit-core.c:277: visit_type_any: Assertion `!err != !*obj'
failed
Clear the obj and return an error.
The patch also fixes a similar potential crash in qmp_input_type_null()
by checking qmp_input_get_object() returned a valid qobj.
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
qapi/qmp-input-visitor.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 64dd392..fc91e74 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -338,6 +338,12 @@ static void qmp_input_type_any(Visitor *v, const char
*name, QObject **obj,
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name, true);
+if (!qobj) {
+error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
+*obj = NULL;
+return;
+}
+
qobject_incref(qobj);
*obj = qobj;
}
@@ -347,6 +353,11 @@ static void qmp_input_type_null(Visitor *v, const char
*name, Error **errp)
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name, true);
+if (!qobj) {
+error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
+return;
+}
+
if (qobject_type(qobj) != QTYPE_QNULL) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"null");
--
2.10.0
