linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Kochetkov <al.kochet@gmail.com>
To: Kevin Hilman <khilman@kernel.org>,
	Tony Lindgren <tony@atomide.com>, Felipe Balbi <balbi@ti.com>,
	Wolfram Sang <wsa@the-dreams.de>,
	Alexander Kochetkov <al.kochet@gmail.com>
Cc: linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing
Date: Sun, 30 Nov 2014 01:00:02 +0400	[thread overview]
Message-ID: <1417294803-14729-2-git-send-email-al.kochet@gmail.com> (raw)
In-Reply-To: <1417294803-14729-1-git-send-email-al.kochet@gmail.com>

commit dd74548ddece4b9d68e5528287a272fa552c81d0 ("i2c: omap:
resize fifos before each message") dropped check for dev->buf_len.
As result, data processing loop cause dev->buf overruns for
devices with 16-bit data register (omap2420).

In the dd74548ddece4b9d68 code, for each loop iteration if the
flag OMAP_I2C_FLAG_16BIT_DATA_REG is set (omap2420), dev->buf
is incremented twice, and dev->buf_len decremented twice.

Also buffer overrun could happen (in theory) due to wrong
ISR handling (bug).

The commit fix data processing for omap2420 and add guard checks
in the data processing loops do disallow accesses to the buffer,
when dev->buf_len is zero. Also added warnings to unhide the bug.

Found by code review.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
Fixes: dd74548ddece4b9d68e5528287a272fa552c81d0 "i2c: omap: resize fifos before each message"
Reported-by: Tony Lindgren <tony@atomide.com>
---
 drivers/i2c/busses/i2c-omap.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4563200..e890295 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -938,20 +938,30 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)
 static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
 		bool is_rdr)
 {
-	u16		w;
+	u16 w;
+
+	if (unlikely(num_bytes > dev->buf_len)) {
+		dev_err(dev->dev, "%s interrupt can't receive %u byte(s)\n",
+			is_rdr ? "RDR" : "RRDY", (num_bytes - dev->buf_len));
+		num_bytes = dev->buf_len;
+	}
 
-	while (num_bytes--) {
+	while (num_bytes) {
 		w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
 		*dev->buf++ = w;
 		dev->buf_len--;
+		num_bytes--;
 
 		/*
 		 * Data reg in 2430, omap3 and
 		 * omap4 is 8 bit wide
 		 */
 		if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
-			*dev->buf++ = w >> 8;
-			dev->buf_len--;
+			if (num_bytes) {
+				*dev->buf++ = w >> 8;
+				dev->buf_len--;
+				num_bytes--;
+			}
 		}
 	}
 }
@@ -959,19 +969,29 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
 static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,
 		bool is_xdr)
 {
-	u16		w;
+	u16 w;
+
+	if (unlikely(num_bytes > dev->buf_len)) {
+		dev_err(dev->dev, "%s interrupt can't transmit %u byte(s)\n",
+			is_xdr ? "XDR" : "XRDY", (num_bytes - dev->buf_len));
+		num_bytes = dev->buf_len;
+	}
 
-	while (num_bytes--) {
+	while (num_bytes) {
 		w = *dev->buf++;
 		dev->buf_len--;
+		num_bytes--;
 
 		/*
 		 * Data reg in 2430, omap3 and
 		 * omap4 is 8 bit wide
 		 */
 		if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
-			w |= *dev->buf++ << 8;
-			dev->buf_len--;
+			if (num_bytes) {
+				w |= *dev->buf++ << 8;
+				dev->buf_len--;
+				num_bytes--;
+			}
 		}
 
 		if (dev->errata & I2C_OMAP_ERRATA_I462) {
-- 
1.7.9.5

  reply	other threads:[~2014-11-29 21:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov
2014-11-29 21:00 ` Alexander Kochetkov [this message]
     [not found]   ` <1417294803-14729-2-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-12-01 19:58     ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Tony Lindgren
     [not found]       ` <20141201195841.GD2817-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2014-12-02 11:17         ` Alexander Kochetkov
     [not found]           ` <B3F4FEFF-2333-473D-8A35-7D4EF1C44D90-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-12-04 18:09             ` Tony Lindgren
     [not found]               ` <20141204180946.GG2817-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2014-12-05 17:18                 ` Alexander Kochetkov
2014-11-29 21:00 ` [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event Alexander Kochetkov
     [not found] ` <1417294803-14729-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-12-01 20:04   ` [RFC 0/2] i2c: omap: new fixes for driver Kevin Hilman
     [not found]     ` <7hoarn3zme.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
2014-12-02 11:21       ` Alexander Kochetkov
2015-01-13 10:04   ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1417294803-14729-2-git-send-email-al.kochet@gmail.com \
    --to=al.kochet@gmail.com \
    --cc=balbi@ti.com \
    --cc=khilman@kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=tony@atomide.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).