Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752247AbcJJGij (ORCPT ); Mon, 10 Oct 2016 02:38:39 -0400 Received: from mout02.posteo.de ([185.67.36.66]:35612 "EHLO mout02.posteo.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751214AbcJJGih (ORCPT ); Mon, 10 Oct 2016 02:38:37 -0400 Date: Mon, 10 Oct 2016 08:37:44 +0200 From: Patrick Boettcher To: Mauro Carvalho Chehab Cc: Linux Media Mailing List , Mauro Carvalho Chehab , Andy Lutomirski , Johannes Stezenbach , Jiri Kosina , Linux Kernel Mailing List , Andy Lutomirski , Michael Krufky , Mauro Carvalho Chehab , =?UTF-8?B?SsO2cmc=?= Otte , Hans Verkuil , Sean Young , Andrew Morton , Kees Cook , Wolfram Sang Subject: Re: [PATCH 08/26] dib0700_core: don't use stack on I2C reads Message-ID: <20161010083744.1fc6171a@posteo.de> In-Reply-To: References: X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2715 Lines: 86 On Fri, 7 Oct 2016 14:24:18 -0300 Mauro Carvalho Chehab wrote: > Be sure that I2C reads won't use stack by passing > a pointer to the state buffer, that we know it was > allocated via kmalloc, instead of relying on the buffer > allocated by an I2C client. > > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/usb/dvb-usb/dib0700_core.c | 27 > ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 > deletion(-) > > diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c > b/drivers/media/usb/dvb-usb/dib0700_core.c index > 515f89dba199..92d5408684ac 100644 --- > a/drivers/media/usb/dvb-usb/dib0700_core.c +++ > b/drivers/media/usb/dvb-usb/dib0700_core.c @@ -213,7 +213,7 @@ static > int dib0700_i2c_xfer_new(struct i2c_adapter *adap, struct i2c_msg > *msg, usb_rcvctrlpipe(d->udev, 0), REQUEST_NEW_I2C_READ, > USB_TYPE_VENDOR | > USB_DIR_IN, > - value, index, > msg[i].buf, > + value, index, > st->buf, msg[i].len, > USB_CTRL_GET_TIMEOUT); > if (result < 0) { > @@ -221,6 +221,14 @@ static int dib0700_i2c_xfer_new(struct > i2c_adapter *adap, struct i2c_msg *msg, break; > } > > + if (msg[i].len > sizeof(st->buf)) { > + deb_info("buffer too small to fit %d > bytes\n", > + msg[i].len); > + return -EIO; > + } > + > + memcpy(msg[i].buf, st->buf, msg[i].len); > + > deb_data("<<< "); > debug_dump(msg[i].buf, msg[i].len, deb_data); > > @@ -238,6 +246,13 @@ static int dib0700_i2c_xfer_new(struct > i2c_adapter *adap, struct i2c_msg *msg, /* I2C ctrl + FE bus; */ > st->buf[3] = ((gen_mode << 6) & 0xC0) | > ((bus_mode << 4) & 0x30); > + > + if (msg[i].len > sizeof(st->buf) - 4) { > + deb_info("i2c message to big: %d\n", > + msg[i].len); > + return -EIO; > + } > + > /* The Actual i2c payload */ > memcpy(&st->buf[4], msg[i].buf, msg[i].len); > > @@ -283,6 +298,11 @@ static int dib0700_i2c_xfer_legacy(struct > i2c_adapter *adap, /* fill in the address */ > st->buf[1] = msg[i].addr << 1; > /* fill the buffer */ > + if (msg[i].len > sizeof(st->buf) - 2) { > + deb_info("i2c xfer to big: %d\n", > + msg[i].len); > + return -EIO; > + } > memcpy(&st->buf[2], msg[i].buf, msg[i].len); > > /* write/read request */ > @@ -299,6 +319,11 @@ static int dib0700_i2c_xfer_legacy(struct > i2c_adapter *adap, break; > } > > + if (msg[i + 1].len > sizeof(st->buf)) { > + deb_info("i2c xfer buffer to small > for %d\n", > + msg[i].len); > + return -EIO; > + } > memcpy(msg[i + 1].buf, st->buf, msg[i + > 1].len); > msg[i+1].len = len; Reviewed-By: Patrick Boettcher