linux-kernel - [PATCH] i2c: davinci: Fix race when setting up for TX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 22 Jun 2010 16:31:03 +0900
From:	Jon Povey <jon.povey@...elogic.co.uk>
To:	davinci-linux-open-source@...ux.davincidsp.com
Cc:	Jon Povey <jon.povey@...elogic.co.uk>,
	"Jean Delvare (PC drivers, core)" <khali@...ux-fr.org>,
	"Ben Dooks (embedded platforms)" <ben-linux@...ff.org>,
	Kevin Hilman <khilman@...prootsystems.com>,
	Chaithrika U S <chaithrika@...com>,
	Philby John <pjohn@...mvista.com>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] i2c: davinci: Fix race when setting up for TX

Move the interrupt enable after the first byte load for TX, as it
was racing with the interrupt handler and corrupting dev->buf_len
for messages > 1 byte.

Tested on DM355.

Signed-off-by: Jon Povey <jon.povey@...elogic.co.uk>
---
The race seems to have been introduced by 869e6692d527983958216f13c3c8e095791909df

This fixes the problem for me, but someone from TI might want to comment
on the validity of starting the transfer before enabling the RDY interrupts.
Any possibility of losing interrupts?

Considered a spinlock but if this reordering is OK then it's neater.

I'm guessing not many people have been sending multi-byte messages..

 drivers/i2c/busses/i2c-davinci.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 2222c87..72df4af 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -347,14 +347,6 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 		flag &= ~DAVINCI_I2C_MDR_STP;
 	}
 
-	/* Enable receive or transmit interrupts */
-	w = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
-	if (msg->flags & I2C_M_RD)
-		w |= DAVINCI_I2C_IMR_RRDY;
-	else
-		w |= DAVINCI_I2C_IMR_XRDY;
-	davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, w);
-
 	dev->terminate = 0;
 
 	/* write the data into mode register */
@@ -371,6 +363,14 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 		dev->buf_len--;
 	}
 
+	/* Enable receive or transmit interrupts */
+	w = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
+	if (msg->flags & I2C_M_RD)
+		w |= DAVINCI_I2C_IMR_RRDY;
+	else
+		w |= DAVINCI_I2C_IMR_XRDY;
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, w);
+
 	r = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
 						      dev->adapter.timeout);
 	if (r == 0) {
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/