From patchwork Mon Mar 23 17:50:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Abbott X-Patchwork-Id: 6074721 Return-Path: X-Original-To: patchwork-linux-spi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3DF86BF90F for ; Mon, 23 Mar 2015 17:50:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5DB7F2034A for ; Mon, 23 Mar 2015 17:50:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 858E920320 for ; Mon, 23 Mar 2015 17:50:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753062AbbCWRuw (ORCPT ); Mon, 23 Mar 2015 13:50:52 -0400 Received: from smtp85.iad3a.emailsrvr.com ([173.203.187.85]:49187 "EHLO smtp85.iad3a.emailsrvr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752519AbbCWRuw (ORCPT ); Mon, 23 Mar 2015 13:50:52 -0400 Received: from smtp19.relay.iad3a.emailsrvr.com (localhost.localdomain [127.0.0.1]) by smtp19.relay.iad3a.emailsrvr.com (SMTP Server) with ESMTP id 4F7CD18038F; Mon, 23 Mar 2015 13:50:51 -0400 (EDT) Received: by smtp19.relay.iad3a.emailsrvr.com (Authenticated sender: abbotti-AT-mev.co.uk) with ESMTPSA id F1AF0180580; Mon, 23 Mar 2015 13:50:49 -0400 (EDT) X-Sender-Id: abbotti@mev.co.uk Received: from ian-deb.local.mev.co.uk (host86-149-98-55.range86-149.btcentralplus.com [86.149.98.55]) (using TLSv1.2 with cipher AES128-SHA256) by 0.0.0.0:465 (trex/5.4.2); Mon, 23 Mar 2015 17:50:51 GMT From: Ian Abbott To: Cc: Mark Brown , Dan Carpenter , , , Ian Abbott Subject: [PATCH] spi: spidev: fix possible arithmetic overflow for multi-transfer message Date: Mon, 23 Mar 2015 17:50:27 +0000 Message-Id: <1427133027-8134-1-git-send-email-abbotti@mev.co.uk> X-Mailer: git-send-email 2.1.4 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP `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 Cc: # 4.0+ --- This could be backported to kernels prior to 4.0, but the total and individual lengths would need to be checked against `bufsiz` instead of `INT_MAX`. --- drivers/spi/spidev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index bb6b3ab..23ad978 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -249,9 +249,10 @@ static int spidev_message(struct spidev_data *spidev, total += k_tmp->len; /* Since the function returns the total length of transfers * on success, restrict the total to positive int values to - * avoid the return value looking like an error. + * avoid the return value looking like an error. Also check + * each transfer length to avoid arithmetic overflow. */ - if (total > INT_MAX) { + if (total > INT_MAX || k_tmp->len > INT_MAX) { status = -EMSGSIZE; goto done; }