From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga03-in.huawei.com ([119.145.14.66]:37054 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752474AbbFODgk (ORCPT ); Sun, 14 Jun 2015 23:36:40 -0400 Message-ID: <557E4841.6080708@huawei.com> Date: Mon, 15 Jun 2015 11:36:33 +0800 From: Zefan Li MIME-Version: 1.0 To: Ian Abbott CC: Subject: Re: [PATCH 2.6.22 to 4.0] spi: spidev: fix possible arithmetic overflow for multi-transfer message References: <1429286534-17556-1-git-send-email-abbotti@mev.co.uk> In-Reply-To: <1429286534-17556-1-git-send-email-abbotti@mev.co.uk> Content-Type: text/plain; charset="GB2312" Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 2015/4/18 0:02, Ian Abbott wrote: > commit f20fbaad7620 ("spi: spidev: fix possible arithmetic overflow for multi-transfer message") > Queued up for 3.4. Thanks! > `spidev_message()` sums the lengths of the individual SPI transfers to > determine the overall SPI message length. It restricts the total > length, returning an error if too long, but it does not check for > arithmetic overflow. For example, if the SPI message consisted of two > transfers and the first has a length of 10 and the second has a length > of (__u32)(-1), the total length would be seen as 9, even though the > second transfer is actually very long. If the second transfer specifies > a null `rx_buf` and a non-null `tx_buf`, the `copy_from_user()` could > overrun the spidev's pre-allocated tx buffer before it reaches an > invalid user memory address. Fix it by checking that neither the total > nor the individual transfer lengths exceed the maximum allowed value. > > Thanks to Dan Carpenter for reporting the potential integer overflow. > > Signed-off-by: Ian Abbott > --- > Note: original commit compares the lengths to INT_MAX instead of bufsiz > due to changes in earlier commits. > --- > drivers/spi/spidev.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c > index 4eb7a98..7bf5186 100644 > --- a/drivers/spi/spidev.c > +++ b/drivers/spi/spidev.c > @@ -245,7 +245,10 @@ static int spidev_message(struct spidev_data *spidev, > k_tmp->len = u_tmp->len; > > total += k_tmp->len; > - if (total > bufsiz) { > + /* Check total length of transfers. Also check each > + * transfer length to avoid arithmetic overflow. > + */ > + if (total > bufsiz || k_tmp->len > bufsiz) { > status = -EMSGSIZE; > goto done; > } >