From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjNPO-0002LD-9l for qemu-devel@nongnu.org; Mon, 12 Sep 2016 05:20:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjNPI-0002v5-0H for qemu-devel@nongnu.org; Mon, 12 Sep 2016 05:20:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55372) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjNPH-0002uw-PA for qemu-devel@nongnu.org; Mon, 12 Sep 2016 05:20:03 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 67A4180B56 for ; Mon, 12 Sep 2016 09:20:03 +0000 (UTC) From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 12 Sep 2016 13:19:08 +0400 Message-Id: <20160912091913.15831-14-marcandre.lureau@redhat.com> In-Reply-To: <20160912091913.15831-1-marcandre.lureau@redhat.com> References: <20160912091913.15831-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH v6 13/18] qapi: check invalid arguments on no-args commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: eblake@redhat.com, armbru@redhat.com, =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= The generated marshal functions do not visit arguments from commands that take no arguments. Thus they fail to catch invalid members. Visit the arguments, if provided, to throw an error in case of invalid members. Currently, qmp_check_client_args() checks for invalid arguments and correctly catches this case. When switching to qmp_dispatch() we want to keep that behaviour. The commands using 'O' may have arbitrary arguments, and must have 'gen': false in the qapi schema to skip the generated checks. Old/new diff: void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp) { Error *err =3D NULL; + Visitor *v =3D NULL; - (void)args; + if (args) { + v =3D qmp_input_visitor_new(QOBJECT(args), true); + visit_start_struct(v, NULL, NULL, 0, &err); + if (err) { + goto out; + } + + if (!err) { + visit_check_struct(v, &err); + } + visit_end_struct(v, NULL); + if (err) { + goto out; + } + } qmp_stop(&err); + +out: error_propagate(errp, err); + visit_free(v); + if (args) { + v =3D qapi_dealloc_visitor_new(); + visit_start_struct(v, NULL, NULL, 0, NULL); + + visit_end_struct(v, NULL); + visit_free(v); + } } The new code closely resembles code for a command with arguments. Differences: - the visit of the argument and its cleanup struct don't visit any members (because there are none). - the visit of the argument struct and its cleanup are conditional. Signed-off-by: Marc-Andr=C3=A9 Lureau --- tests/test-qmp-commands.c | 15 ++++++++++++ scripts/qapi-commands.py | 58 +++++++++++++++++++++++++++++++++++------= ------ 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 261fd9e..81cbe54 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -106,6 +106,7 @@ static void test_dispatch_cmd(void) static void test_dispatch_cmd_failure(void) { QDict *req =3D qdict_new(); + QDict *args =3D qdict_new(); QObject *resp; =20 qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd= 2"))); @@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void) assert(resp !=3D NULL); assert(qdict_haskey(qobject_to_qdict(resp), "error")); =20 + qobject_decref(resp); + QDECREF(req); + + /* check that with extra arguments it throws an error */ + req =3D qdict_new(); + qdict_put(args, "a", qint_from_int(66)); + qdict_put(req, "arguments", args); + + qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd= "))); + + resp =3D qmp_dispatch(QOBJECT(req)); + assert(resp !=3D NULL); + assert(qdict_haskey(qobject_to_qdict(resp), "error")); + qobject_decref(resp); QDECREF(req); } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index eac64ce..2f603b0 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -95,6 +95,8 @@ def gen_marshal_decl(name): =20 =20 def gen_marshal(name, arg_type, boxed, ret_type): + have_args =3D arg_type and not arg_type.is_empty() + ret =3D mcgen(''' =20 %(proto)s @@ -109,17 +111,31 @@ def gen_marshal(name, arg_type, boxed, ret_type): ''', c_type=3Dret_type.c_type()) =20 - if arg_type and not arg_type.is_empty(): + if have_args: + visit_members =3D ('visit_type_%s_members(v, &arg, &err);' + % arg_type.c_name()) ret +=3D mcgen(''' Visitor *v; %(c_name)s arg =3D {0}; =20 +''', + c_name=3Darg_type.c_name()) + else: + visit_members =3D '' + ret +=3D mcgen(''' + Visitor *v =3D NULL; + + if (args) { +''') + push_indent() + + ret +=3D mcgen(''' v =3D qmp_input_visitor_new(QOBJECT(args), true); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; } - visit_type_%(c_name)s_members(v, &arg, &err); + %(visit_members)s if (!err) { visit_check_struct(v, &err); } @@ -128,35 +144,47 @@ def gen_marshal(name, arg_type, boxed, ret_type): goto out; } ''', - c_name=3Darg_type.c_name()) + visit_members=3Dvisit_members) =20 - else: + if not have_args: + pop_indent() ret +=3D mcgen(''' - - (void)args; + } ''') =20 ret +=3D gen_call(name, arg_type, boxed, ret_type) =20 - # 'goto out' produced above for arg_type, and by gen_call() for ret_= type - if (arg_type and not arg_type.is_empty()) or ret_type: - ret +=3D mcgen(''' + ret +=3D mcgen(''' =20 out: -''') - ret +=3D mcgen(''' error_propagate(errp, err); + visit_free(v); ''') - if arg_type and not arg_type.is_empty(): + + if have_args: + visit_members =3D ('visit_type_%s_members(v, &arg, NULL);' + % arg_type.c_name()) + else: + visit_members =3D '' ret +=3D mcgen(''' - visit_free(v); + if (args) { +''') + push_indent() + + ret +=3D mcgen(''' v =3D qapi_dealloc_visitor_new(); visit_start_struct(v, NULL, NULL, 0, NULL); - visit_type_%(c_name)s_members(v, &arg, NULL); + %(visit_members)s visit_end_struct(v, NULL); visit_free(v); ''', - c_name=3Darg_type.c_name()) + visit_members=3Dvisit_members) + + if not have_args: + pop_indent() + ret +=3D mcgen(''' + } +''') =20 ret +=3D mcgen(''' } --=20 2.10.0