linux-kernel - [PATCH 1/4] i2c: rtl9300: Fix multi-byte I2C write

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250802-i2c-rtl9300-multi-byte-v1-1-5f687e0098e2@narfation.org>
Date: Sat, 02 Aug 2025 12:32:00 +0200
From: Sven Eckelmann <sven@...fation.org>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>, 
 Andi Shyti <andi.shyti@...nel.org>
Cc: linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Jonas Jelonek <jelonek.jonas@...il.com>, 
 Harshal Gohel <hg@...onwunderlich.de>, 
 Simon Wunderlich <sw@...onwunderlich.de>, 
 Sven Eckelmann <sven@...fation.org>, stable@...r.kernel.org
Subject: [PATCH 1/4] i2c: rtl9300: Fix multi-byte I2C write

From: Harshal Gohel <hg@...onwunderlich.de>

The RTL93xx I2C controller has 4 32 bit registers to store the bytes for
the upcoming I2C transmission. The first byte is stored in the
least-significant byte of the first register. And the last byte in the most
significant byte of the last register. A map of the transferred bytes to
their order in the registers is:

reg 0: 04-03-02-01
reg 1: 08-07-06-05
reg 2: 0c-0b-0a-09
reg 3: 10-0f-0e-0d

The i2c_read() function basically demonstrates how the hardware would pick
up bytes from this register set. But the i2c_write() function was just
pushing bytes one after another to the least significant byte of a register
AFTER shifting the last one to the next more significant byte position.

If you would then have tried to send a buffer with numbers 1-11 using
i2c_write(), you would have ended up with following register content:

reg 0: 01-02-03-04
reg 1: 05-06-07-08
reg 2: 00-09-0a-0b
reg 3: 00-00-00-00

On the wire, you would then have seen:

  Sr Addr Rd/Wr [A] 04 A 03 A 02 A 01 A 08 A 07 A 06 A 05 A 0b A 0a A 09 A/NA P

But the correct data transmission was expected to be

  Sr Addr Rd/Wr [A] 01 A 02 A 03 A 04 A 05 A 06 A 07 A 08 A 09 A 0a A 0b A/NA P

Because of this multi-byte ordering problem, only single byte i2c_write()
operations were executed correctly (on the wire).

By shifting the byte directly to the correct end position in the register,
it is possible to avoid this incorrect byte ordering and fix multi-byte
transmissions.

Cc: <stable@...r.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Harshal Gohel <hg@...onwunderlich.de>
Co-developed-by: Sven Eckelmann <sven@...fation.org>
Signed-off-by: Sven Eckelmann <sven@...fation.org>
---
 drivers/i2c/busses/i2c-rtl9300.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index e064e8a4a1f0824abc82fa677866b85f99fbe4a7..1b3cbe3ea84a4fa480c5c00438eecc551d047348 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -143,10 +143,13 @@ static int rtl9300_i2c_write(struct rtl9300_i2c *i2c, u8 *buf, int len)
 		return -EIO;

 	for (i = 0; i < len; i++) {
+		unsigned int shift = (i % 4) * 8;
+		unsigned int reg = i / 4;
+
 		if (i % 4 == 0)
-			vals[i/4] = 0;
-		vals[i/4] <<= 8;
-		vals[i/4] |= buf[i];
+			vals[reg] = 0;
+
+		vals[reg] |= buf[i] << shift;
 	}

 	return regmap_bulk_write(i2c->regmap, i2c->reg_base + RTL9300_I2C_MST_DATA_WORD0,

-- 
2.47.2