From c489780203f9b22aca5539ec7589b7140bdc951f Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Fri, 23 Sep 2016 00:39:26 +0400 Subject: qapi: Fix crash when 'any' or 'null' parameter is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20160922203927.28241-3-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster [Commit message rephrased] Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 64dd392e6f..fc91e74894 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"); -- cgit 1.4.1 From 5d0cbbcfeb59e1e3f5ee7d26b8a215382f6d9abd Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Fri, 30 Sep 2016 13:59:46 +0400 Subject: qapi: add assert about root value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qiv->root should not be null, make that clearer with some assert. Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20160930095948.3154-2-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index fc91e74894..c7deca95b3 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -64,6 +64,7 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, if (QSLIST_EMPTY(&qiv->stack)) { /* Starting at root, name is ignored. */ + assert(qiv->root); return qiv->root; } @@ -395,6 +396,7 @@ Visitor *qmp_input_visitor_new(QObject *obj, bool strict) { QmpInputVisitor *v; + assert(obj); v = g_malloc0(sizeof(*v)); v->visitor.type = VISITOR_INPUT; -- cgit 1.4.1 From eac8e79ff749fc15e1dca4caccf1f38664ab4915 Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Fri, 30 Sep 2016 13:59:47 +0400 Subject: qapi: assert list entry has a value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This helps to figure out the expectations. Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20160930095948.3154-3-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 1 + 1 file changed, 1 insertion(+) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index c7deca95b3..fe097c99b3 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -84,6 +84,7 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, assert(qobject_type(qobj) == QTYPE_QLIST); assert(!name); ret = qlist_entry_obj(tos->entry); + assert(ret); if (consume) { tos->entry = qlist_next(tos->entry); } -- cgit 1.4.1 From 1382d4abdf9619985e4078e37e49e487cea9935e Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Fri, 30 Sep 2016 13:59:48 +0400 Subject: qapi: return a 'missing parameter' error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'old' dispatch code returned a QERR_MISSING_PARAMETER for missing parameters, but the qapi qmp_dispatch() code uses QERR_INVALID_PARAMETER_TYPE. Improve qapi code to return QERR_MISSING_PARAMETER where appropriate. Fix expected error message in iotests. Signed-off-by: Marc-André Lureau Reviewed-by: Alberto Garcia Reviewed-by: Eric Blake Message-Id: <20160930095948.3154-4-marcandre.lureau@redhat.com> [Drop incorrect error_setg() from qmp_input_type_any() and qmp_input_type_null()] Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 67 +++++++++++++++++++++++++++++++++------------- tests/qemu-iotests/087.out | 2 +- 2 files changed, 49 insertions(+), 20 deletions(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index fe097c99b3..37a8e1f931 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -56,7 +56,7 @@ static QmpInputVisitor *to_qiv(Visitor *v) static QObject *qmp_input_get_object(QmpInputVisitor *qiv, const char *name, - bool consume) + bool consume, Error **errp) { StackObject *tos; QObject *qobj; @@ -80,6 +80,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, bool removed = g_hash_table_remove(tos->h, name); assert(removed); } + if (!ret) { + error_setg(errp, QERR_MISSING_PARAMETER, name); + } } else { assert(qobject_type(qobj) == QTYPE_QLIST); assert(!name); @@ -165,13 +168,16 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, true); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); Error *err = NULL; if (obj) { *obj = NULL; } - if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { + if (!qobj) { + return; + } + if (qobject_type(qobj) != QTYPE_QDICT) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict"); return; @@ -193,10 +199,13 @@ static void qmp_input_start_list(Visitor *v, const char *name, GenericList **list, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, true); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); const QListEntry *entry; - if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { + if (!qobj) { + return; + } + if (qobject_type(qobj) != QTYPE_QLIST) { if (list) { *list = NULL; } @@ -234,11 +243,10 @@ static void qmp_input_start_alternate(Visitor *v, const char *name, bool promote_int, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, false); + QObject *qobj = qmp_input_get_object(qiv, name, false, errp); if (!qobj) { *obj = NULL; - error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); return; } *obj = g_malloc0(size); @@ -252,8 +260,13 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); + QInt *qint; + if (!qobj) { + return; + } + qint = qobject_to_qint(qobj); if (!qint) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer"); @@ -268,8 +281,13 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, { /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); + QInt *qint; + if (!qobj) { + return; + } + qint = qobject_to_qint(qobj); if (!qint) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer"); @@ -283,8 +301,13 @@ static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); + QBool *qbool; + if (!qobj) { + return; + } + qbool = qobject_to_qbool(qobj); if (!qbool) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean"); @@ -298,10 +321,15 @@ static void qmp_input_type_str(Visitor *v, const char *name, char **obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); + QString *qstr; + *obj = NULL; + if (!qobj) { + return; + } + qstr = qobject_to_qstring(qobj); if (!qstr) { - *obj = NULL; error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string"); return; @@ -314,10 +342,13 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, true); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); QInt *qint; QFloat *qfloat; + if (!qobj) { + return; + } qint = qobject_to_qint(qobj); if (qint) { *obj = qint_get_int(qobject_to_qint(qobj)); @@ -338,11 +369,10 @@ static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, true); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); + *obj = NULL; if (!qobj) { - error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); - *obj = NULL; return; } @@ -353,10 +383,9 @@ static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj, 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); + QObject *qobj = qmp_input_get_object(qiv, name, true, errp); if (!qobj) { - error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); return; } @@ -369,7 +398,7 @@ static void qmp_input_type_null(Visitor *v, const char *name, Error **errp) static void qmp_input_optional(Visitor *v, const char *name, bool *present) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name, false); + QObject *qobj = qmp_input_get_object(qiv, name, false, NULL); if (!qobj) { *present = false; diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out index cd02eaed4c..dc6baf9366 100644 --- a/tests/qemu-iotests/087.out +++ b/tests/qemu-iotests/087.out @@ -56,7 +56,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on Testing: -S QMP_VERSION {"return": {}} -{"error": {"class": "GenericError", "desc": "Invalid parameter type for 'driver', expected: string"}} +{"error": {"class": "GenericError", "desc": "Parameter 'driver' is missing"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} -- cgit 1.4.1