Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933565AbbG1KYf (ORCPT ); Tue, 28 Jul 2015 06:24:35 -0400 Received: from mail-ob0-f176.google.com ([209.85.214.176]:35262 "EHLO mail-ob0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932370AbbG1KX1 convert rfc822-to-8bit (ORCPT ); Tue, 28 Jul 2015 06:23:27 -0400 MIME-Version: 1.0 In-Reply-To: <20150727210619.GA2825@dtor-ws> References: <20150727210619.GA2825@dtor-ws> Date: Tue, 28 Jul 2015 12:23:27 +0200 X-Google-Sender-Auth: CxEXkIeZo1j_Sh4dyjsRk4x6o5A Message-ID: Subject: Re: [PATCH] Input: zforce_ts - fix playload length check From: Geert Uytterhoeven To: Dmitry Torokhov Cc: "linux-input@vger.kernel.org" , Dirk Behme , Heiko Stuebner , Oleksij Rempel , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2654 Lines: 66 On Mon, Jul 27, 2015 at 11:06 PM, Dmitry Torokhov wrote: > Commit 7d01cd261c76f95913c81554a751968a1d282d3a ("Input: zforce - don't > overwrite the stack") attempted to add a check for payload size being too > large for the supplied buffer. Unfortunately with the currently selected > buffer size the comparison is always false as buffer size is larger than > the value a single byte can hold, and that results in compiler warnings. > Additionally the check was incorrect as it was not accounting for the > already read 2 bytes of data stored in the buffer. The check was indeed incorrect. > Fixes: 7d01cd261c76f95913c81554a751968a1d282d3a > Reported-by: kbuild test robot > Signed-off-by: Dmitry Torokhov > --- > > This seems to shut up my GCC, I wonder if it is going to work gfor > everyone or we better add BUILD_BUG_ON(FRAME_MAXSIZE < 257) and a > comment and remove check. > > drivers/input/touchscreen/zforce_ts.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c > index 2554efd..542ff02 100644 > --- a/drivers/input/touchscreen/zforce_ts.c > +++ b/drivers/input/touchscreen/zforce_ts.c > @@ -441,7 +441,9 @@ static int zforce_read_packet(struct zforce_ts *ts, u8 *buf) > goto unlock; > } > > - if (buf[PAYLOAD_LENGTH] == 0 || buf[PAYLOAD_LENGTH] > FRAME_MAXSIZE) { > + if (buf[PAYLOAD_LENGTH] == 0 || > + (FRAME_MAXSIZE - 2 < 255 && > + buf[PAYLOAD_LENGTH] > FRAME_MAXSIZE - 2)) { Doesn't help with gcc 4.1.2 :-( Before: drivers/input/touchscreen/zforce_ts.c: In function ‘zforce_read_packet’: drivers/input/touchscreen/zforce_ts.c:432: warning: comparison is always false due to limited range of data type After: drivers/input/touchscreen/zforce_ts.c: In function ‘zforce_read_packet’: drivers/input/touchscreen/zforce_ts.c:434: warning: comparison is always false due to limited range of data type Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/