From patchwork Thu Aug 21 01:39:41 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Roszko X-Patchwork-Id: 4755261 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 38FF89F3B4 for ; Thu, 21 Aug 2014 01:42:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 563F92012F for ; Thu, 21 Aug 2014 01:42:16 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4672B200E7 for ; Thu, 21 Aug 2014 01:42:15 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XKHMP-0004br-6i; Thu, 21 Aug 2014 01:40:17 +0000 Received: from mail-qg0-x22d.google.com ([2607:f8b0:400d:c04::22d]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1XKHML-0003TG-RO for linux-arm-kernel@lists.infradead.org; Thu, 21 Aug 2014 01:40:14 +0000 Received: by mail-qg0-f45.google.com with SMTP id f51so8187399qge.18 for ; Wed, 20 Aug 2014 18:39:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=50MfhZGm39+6NzE1jp4HfhG/bcv5XTjY8U4L7DfWvVA=; b=jjslH1GJtiUXev14jjFwgGr/rI4cPZtV5tix+QII0yjLldZ47xrl8e4CgjadXGfy+F rtQ1LXzdnCU5CNhsFiVLK/H36qh+9YEAL0sOMVy6N/fXL88AFuEdEBNhyDn3gWb1pjPP ZHVmmRM1vqSF7Gc1IrJgDNfXPrvBUcCRVD2KSAwTRBxcOo4K5bhrn9Z/36zBT/Xwx8S1 +SdzmqMj4JfJU+h18OOzz9vQK5BHrPXcYAtvabvRwrxNn/nm5L8wYcqFkbnKml2NfwUZ JcGUoKNbD+HvuinXU4FxlIz6zUM6cWxVMVbpe8gDfQ1qNFNHydJ7TweojOQRAUqfWxy+ DYiQ== X-Received: by 10.140.86.147 with SMTP id p19mr80238591qgd.66.1408585190301; Wed, 20 Aug 2014 18:39:50 -0700 (PDT) Received: from localhost.localdomain (ool-2f142fe1.dyn.optonline.net. [47.20.47.225]) by mx.google.com with ESMTPSA id t3sm43580162qaj.0.2014.08.20.18.39.49 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Aug 2014 18:39:49 -0700 (PDT) From: Marek Roszko To: wsa@the-dreams.de Subject: [PATCH v2] i2c:at91: add bound checking on SMBus block length bytes Date: Wed, 20 Aug 2014 21:39:41 -0400 Message-Id: <1408585181-10131-1-git-send-email-mark.roszko@gmail.com> X-Mailer: git-send-email 1.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140820_184013_967820_CED80A7F X-CRM114-Status: GOOD ( 15.77 ) X-Spam-Score: -0.1 (/) Cc: Marek Roszko , ludovic.desroches@atmel.com, linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, T_DKIM_INVALID, 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 The driver was not bound checking the received length byte to ensure it was within the the buffer size that is allocated for SMBus blocks. This resulted in buffer overflows whenever an invalid length byte was received. It also failed to ensure the length byte was not zero. If it received zero, it would end up in an infinite loop as the at91_twi_read_next_byte function returned immediately without allowing RHR to be read to clear the RXRDY interrupt. Tested agaisnt a SMBus compliant battery. Signed-off-by: Marek Roszko Acked-by: Ludovic Desroches --- Change from v1: fixed typo in commit message reworded message slightly to be specifically say length byte drivers/i2c/busses/i2c-at91.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c index e95f9ba..ec4ff33 100644 --- a/drivers/i2c/busses/i2c-at91.c +++ b/drivers/i2c/busses/i2c-at91.c @@ -101,6 +101,7 @@ struct at91_twi_dev { unsigned twi_cwgr_reg; struct at91_twi_pdata *pdata; bool use_dma; + bool recv_len_abort; struct at91_twi_dma dma; }; @@ -267,12 +268,24 @@ static void at91_twi_read_next_byte(struct at91_twi_dev *dev) *dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff; --dev->buf_len; + /* return if aborting, we only needed to read RHR to clear RXRDY*/ + if (dev->recv_len_abort) + return; + /* handle I2C_SMBUS_BLOCK_DATA */ if (unlikely(dev->msg->flags & I2C_M_RECV_LEN)) { - dev->msg->flags &= ~I2C_M_RECV_LEN; - dev->buf_len += *dev->buf; - dev->msg->len = dev->buf_len + 1; - dev_dbg(dev->dev, "received block length %d\n", dev->buf_len); + /* ensure length byte is a valid value */ + if (*dev->buf <= I2C_SMBUS_BLOCK_MAX && *dev->buf > 0) { + dev->msg->flags &= ~I2C_M_RECV_LEN; + dev->buf_len += *dev->buf; + dev->msg->len = dev->buf_len + 1; + dev_dbg(dev->dev, "received block length %d\n", + dev->buf_len); + } else { + /* abort and send the stop by reading one more byte */ + dev->recv_len_abort = true; + dev->buf_len = 1; + } } /* send stop if second but last byte has been read */ @@ -444,6 +457,12 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev) ret = -EIO; goto error; } + if (dev->recv_len_abort) { + dev_err(dev->dev, "invalid smbus block length recvd\n"); + ret = -EPROTO; + goto error; + } + dev_dbg(dev->dev, "transfer complete\n"); return 0; @@ -500,6 +519,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num) dev->buf_len = m_start->len; dev->buf = m_start->buf; dev->msg = m_start; + dev->recv_len_abort = false; ret = at91_do_twi_transfer(dev);