From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RQ0nq-0003kd-Bo for qemu-devel@nongnu.org; Mon, 14 Nov 2011 12:58:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RQ0np-0005jD-Ao for qemu-devel@nongnu.org; Mon, 14 Nov 2011 12:58:42 -0500 Received: from e31.co.us.ibm.com ([32.97.110.149]:51597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RQ0np-0005ii-1N for qemu-devel@nongnu.org; Mon, 14 Nov 2011 12:58:41 -0500 Received: from /spool/local by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 Nov 2011 10:58:33 -0700 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pAEHwJhp185074 for ; Mon, 14 Nov 2011 10:58:20 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pAEHwFJl007202 for ; Mon, 14 Nov 2011 10:58:15 -0700 Message-ID: <4EC156B6.4060506@linux.vnet.ibm.com> Date: Mon, 14 Nov 2011 11:58:14 -0600 From: Michael Roth MIME-Version: 1.0 References: <20111114152920.37edca6f@doriath> In-Reply-To: <20111114152920.37edca6f@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1.0?] qapi: Check for negative enum values List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel On 11/14/2011 11:29 AM, Luiz Capitulino wrote: > We don't currently check for negative enum values in qmp_output_type_enum(), > this will very likely generate a segfault when triggered. > > However, it _seems_ that no code in tree can trigger this today. > > Signed-off-by: Luiz Capitulino > --- > > I think it's a good idea to merge this one for 1.0, but I'd be ok to queue it > for 1.1 in case we're only merging fixes for "real" bugs. > > I found this while extending the QAPI's unit-tests... > > qapi/qmp-output-visitor.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c > index d67724e..f76d015 100644 > --- a/qapi/qmp-output-visitor.c > +++ b/qapi/qmp-output-visitor.c > @@ -190,7 +190,7 @@ static void qmp_output_type_enum(Visitor *v, int *obj, const char *strings[], > > assert(strings); > while (strings[i++] != NULL); > - if (value>= i - 1) { > + if (value< 0 || value>= i - 1) { > error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null"); > return; > } Acked-by: Michael Roth