* [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations
@ 2025-08-09 6:40 Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer Sven Eckelmann
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann, Alex Guo, stable
During the integration of the RTL8239 POE chip + its frontend MCU, it was
noticed that multi-byte operations were basically broken in the current
driver.
Tests using SMBus Block Writes showed that the data (after the Wr maker +
Ack) was mixed up on the wire. At first glance, it looked like an
endianness problem. But for transfers where the number of count + data
bytes was not divisible by 4, the last bytes were not looking like an
endianness problem because they were in the wrong order but not for example
0 - which would be the case for an endianness problem with 32 bit
registers. At the end, it turned out to be the way how i2c_write tried to
add the bytes to the send registers.
Each 32 bit register was used similar to a shift register - shifting the
various bytes up the register while the next one is added to the least
significant byte. But the I2C controller expects the first byte of the
transmission in the least significant byte of the first register. And the
last byte (assuming it is a 16 byte transfer) is expected in the most
significant byte of the fourth register.
While doing these tests, it was also observed that the count byte was
missing from the SMBus Block Writes. The driver just removed them from the
data->block (from the I2C subsystem). But the I2C controller DOES NOT
automatically add this byte - for example by using the configured
transmission length.
The RTL8239 MCU is not actually an SMBus compliant device. Instead, it
expects I2C Block Reads + I2C Block Writes. But according to the already
identified bugs in the driver, it was clear that the I2C controller can
simply be modified to not send the count byte for I2C_SMBUS_I2C_BLOCK_DATA.
The receive part just needs to write the content of the receive buffer to
the correct position in data->block.
While the on-wire format was now correct, reads were still not possible
against the MCU (for the RTL8239 POE chip). It was always timing out
because the 2ms were not enough for sending the read request and then
receiving the 12 byte answer.
These changes were originally submitted to OpenWrt. But there are plans to
migrate OpenWrt to the upstream Linux driver. As a result, the pull request
was stopped and the changes were redone against this driver.
For reasons of transparency: The work on I2C_SMBUS_I2C_BLOCK_DATA support
for the RTL8239-MCU was done on RTL931xx. All problems were therefore
detected with the patches from Jonas Jelonek [1] and not the vanilla Linux
driver. But looking through the code, it seems like these are NOT
regressions introduced by the RTL931x patchset.
I've picked up Alex Guo's patch [2] to reduce conflicts between pending
fixes.
[1] https://patchwork.ozlabs.org/project/linux-i2c/cover/20250727114800.3046-1-jelonek.jonas@gmail.com/
[2] https://lore.kernel.org/r/20250615235248.529019-1-alexguo1023@gmail.com
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Changes in v4:
- Provide only "write" examples for "i2c: rtl9300: Fix multi-byte I2C write"
- drop the second initialization of vals in rtl9300_i2c_write() directly in
the "Fix multi-byte I2C write" fix
- indicate in target branch for each patch in PATCH prefix
- minor commit message cleanups
- Link to v3: https://lore.kernel.org/r/20250804-i2c-rtl9300-multi-byte-v3-0-e20607e1b28c@narfation.org
Changes in v3:
- integrated patch
https://lore.kernel.org/r/20250615235248.529019-1-alexguo1023@gmail.com
to avoid conflicts in the I2C_SMBUS_BLOCK_DATA code
- added Fixes and stable@vger.kernel.org to Alex Guo's patch
- added Chris Packham's Reviewed-by/Acked-by
- Link to v2: https://lore.kernel.org/r/20250803-i2c-rtl9300-multi-byte-v2-0-9b7b759fe2b6@narfation.org
Changes in v2:
- add the missing transfer width and read length increase for the SMBus
Write/Read
- Link to v1: https://lore.kernel.org/r/20250802-i2c-rtl9300-multi-byte-v1-0-5f687e0098e2@narfation.org
---
Alex Guo (1):
[i2c-host-fixes] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer
Harshal Gohel (2):
[i2c-host-fixes] i2c: rtl9300: Fix multi-byte I2C write
[i2c-host] i2c: rtl9300: Implement I2C block read and write
Sven Eckelmann (2):
[i2c-host-fixes] i2c: rtl9300: Increase timeout for transfer polling
[i2c-host-fixes] i2c: rtl9300: Add missing count byte for SMBus Block Ops
drivers/i2c/busses/i2c-rtl9300.c | 50 +++++++++++++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 8 deletions(-)
---
base-commit: 09eaa2a604ed1bfda7e6fb10488127ce8fdc8048
change-id: 20250802-i2c-rtl9300-multi-byte-edaa1fb0872c
Best regards,
--
Sven Eckelmann <sven@narfation.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
@ 2025-08-09 6:40 ` Sven Eckelmann
2025-08-09 17:34 ` Wolfram Sang
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 2/5] i2c: rtl9300: Fix multi-byte I2C write Sven Eckelmann
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann, Alex Guo, stable
From: Alex Guo <alexguo1023@gmail.com>
The data->block[0] variable comes from user. Without proper check,
the variable may be very large to cause an out-of-bounds bug.
Fix this bug by checking the value of data->block[0] first.
1. commit 39244cc75482 ("i2c: ismt: Fix an out-of-bounds bug in
ismt_access()")
2. commit 92fbb6d1296f ("i2c: xgene-slimpro: Fix out-of-bounds bug in
xgene_slimpro_i2c_xfer()")
Cc: <stable@vger.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Alex Guo <alexguo1023@gmail.com>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/i2c/busses/i2c-rtl9300.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index e064e8a4a1f0824abc82fa677866b85f99fbe4a7..568495720810b373c4fa3b31d3f4cdec7c64b5f9 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -281,6 +281,10 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
if (ret)
goto out_unlock;
+ if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
if (ret)
goto out_unlock;
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH i2c-host-fixes v4 2/5] i2c: rtl9300: Fix multi-byte I2C write
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer Sven Eckelmann
@ 2025-08-09 6:40 ` Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 3/5] i2c: rtl9300: Increase timeout for transfer polling Sven Eckelmann
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann, stable
From: Harshal Gohel <hg@simonwunderlich.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: 0x04_03_02_01
reg 1: 0x08_07_06_05
reg 2: 0x0c_0b_0a_09
reg 3: 0x10_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: 0x01_02_03_04
reg 1: 0x05_06_07_08
reg 2: 0x00_09_0a_0b
reg 3: 0x00_00_00_00
On the wire, you would then have seen:
Sr Addr Wr [A] 04 A 03 A 02 A 01 A 08 A 07 A 06 A 05 A 0b A 0a A 09 A P
But the correct data transmission was expected to be
Sr Addr Wr [A] 01 A 02 A 03 A 04 A 05 A 06 A 07 A 08 A 09 A 0a A 0b A 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.
The second initialization (to 0) of vals was also be dropped because this
array is initialized to 0 on the stack by using `= {};`. This makes the
fix a lot more readable.
Cc: <stable@vger.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Harshal Gohel <hg@simonwunderlich.de>
Co-developed-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-rtl9300.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index 568495720810b373c4fa3b31d3f4cdec7c64b5f9..4a538b2660802c98e1a51d2ed782e154f2a5d1a0 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -143,10 +143,10 @@ static int rtl9300_i2c_write(struct rtl9300_i2c *i2c, u8 *buf, int len)
return -EIO;
for (i = 0; i < len; i++) {
- if (i % 4 == 0)
- vals[i/4] = 0;
- vals[i/4] <<= 8;
- vals[i/4] |= buf[i];
+ unsigned int shift = (i % 4) * 8;
+ unsigned int reg = i / 4;
+
+ vals[reg] |= buf[i] << shift;
}
return regmap_bulk_write(i2c->regmap, i2c->reg_base + RTL9300_I2C_MST_DATA_WORD0,
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH i2c-host-fixes v4 3/5] i2c: rtl9300: Increase timeout for transfer polling
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 2/5] i2c: rtl9300: Fix multi-byte I2C write Sven Eckelmann
@ 2025-08-09 6:40 ` Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 4/5] i2c: rtl9300: Add missing count byte for SMBus Block Ops Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write Sven Eckelmann
4 siblings, 0 replies; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann, stable
The timeout for transfers was only set to 2ms. Because of this relatively
low limit, 12-byte read operations to the frontend MCU of a RTL8239 POE PSE
chip cluster was consistently resulting in a timeout.
The original OpenWrt downstream driver [1] was not using any timeout limit
at all. This is also possible by setting the timeout_us parameter of
regmap_read_poll_timeout() to 0. But since the driver currently implements
the ETIMEDOUT error, it is more sensible to increase the timeout in such a
way that communication with the (quite common) Realtek I2C-connected POE
management solution is possible.
[1] https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/realtek/files-6.12/drivers/i2c/busses/i2c-rtl9300.c;h=c4d973195ef39dc56d6207e665d279745525fcac#l202
Cc: <stable@vger.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-rtl9300.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index 4a538b2660802c98e1a51d2ed782e154f2a5d1a0..4a282d57e2c1a72c95bdabdd9eb348a73df28c44 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -175,7 +175,7 @@ static int rtl9300_i2c_execute_xfer(struct rtl9300_i2c *i2c, char read_write,
return ret;
ret = regmap_read_poll_timeout(i2c->regmap, i2c->reg_base + RTL9300_I2C_MST_CTRL1,
- val, !(val & RTL9300_I2C_MST_CTRL1_I2C_TRIG), 100, 2000);
+ val, !(val & RTL9300_I2C_MST_CTRL1_I2C_TRIG), 100, 100000);
if (ret)
return ret;
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH i2c-host-fixes v4 4/5] i2c: rtl9300: Add missing count byte for SMBus Block Ops
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
` (2 preceding siblings ...)
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 3/5] i2c: rtl9300: Increase timeout for transfer polling Sven Eckelmann
@ 2025-08-09 6:40 ` Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write Sven Eckelmann
4 siblings, 0 replies; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann, stable
The expected on-wire format of an SMBus Block Write is
S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P
Everything starting from the Count byte is provided by the I2C subsystem in
the array data->block. But the driver was skipping the Count byte
(data->block[0]) when sending it to the RTL93xx I2C controller.
Only the actual data could be seen on the wire:
S Addr Wr [A] Comm [A] Data [A] Data [A] ... [A] Data [A] P
This wire format is not SMBus Block Write compatible but matches the format
of an I2C Block Write. Simply adding the count byte to the buffer for the
I2C controller is enough to fix the transmission.
This also affects read because the I2C controller must receive the count
byte + $count * data bytes.
Cc: <stable@vger.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-rtl9300.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index 4a282d57e2c1a72c95bdabdd9eb348a73df28c44..cfafe089102aa208dde37096d5105d4140278ca9 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -285,15 +285,15 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
ret = -EINVAL;
goto out_unlock;
}
- ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
+ ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0] + 1);
if (ret)
goto out_unlock;
if (read_write == I2C_SMBUS_WRITE) {
- ret = rtl9300_i2c_write(i2c, &data->block[1], data->block[0]);
+ ret = rtl9300_i2c_write(i2c, &data->block[0], data->block[0] + 1);
if (ret)
goto out_unlock;
}
- len = data->block[0];
+ len = data->block[0] + 1;
break;
default:
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
` (3 preceding siblings ...)
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 4/5] i2c: rtl9300: Add missing count byte for SMBus Block Ops Sven Eckelmann
@ 2025-08-09 6:40 ` Sven Eckelmann
2025-08-09 22:11 ` Jonas Jelonek
4 siblings, 1 reply; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-09 6:40 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Jonas Jelonek, Harshal Gohel,
Simon Wunderlich, Sven Eckelmann
From: Harshal Gohel <hg@simonwunderlich.de>
It was noticed that the original implementation of SMBus Block Write in the
driver was actually an I2C Block Write. Both differ only in the Count byte
before the actual data:
S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P
The I2C Block Write is just skipping this Count byte and starts directly
with the data:
S Addr Wr [A] Comm [A] Data [A] Data [A] ... [A] Data [A] P
The I2C controller of RTL93xx doesn't handle this Count byte special and it
is simply another one of (16 possible) data bytes. Adding support for the
I2C Block Write therefore only requires skipping the count byte (0) in
data->block.
It is similar for reads. The SMBUS Block read is having a Count byte before
the data:
S Addr Wr [A] Comm [A]
Sr Addr Rd [A] [Count] A [Data] A [Data] A ... A [Data] NA P
And the I2C Block Read is directly starting with the actual data:
S Addr Wr [A] Comm [A]
Sr Addr Rd [A] [Data] A [Data] A ... A [Data] NA P
The I2C controller is also not handling this byte in a special way. It
simply provides every byte after the Rd marker + Ack as part of the 16 byte
receive buffer (registers). The content of this buffer just has to be
copied to the right position in the receive data->block.
Signed-off-by: Harshal Gohel <hg@simonwunderlich.de>
Co-developed-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-rtl9300.c | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index cfafe089102aa208dde37096d5105d4140278ca9..2ccc139ffbf606ed825042818e1a3edaf06a7b8f 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -183,22 +183,32 @@ static int rtl9300_i2c_execute_xfer(struct rtl9300_i2c *i2c, char read_write,
return -EIO;
if (read_write == I2C_SMBUS_READ) {
- if (size == I2C_SMBUS_BYTE || size == I2C_SMBUS_BYTE_DATA) {
+ switch (size) {
+ case I2C_SMBUS_BYTE:
+ case I2C_SMBUS_BYTE_DATA:
ret = regmap_read(i2c->regmap,
i2c->reg_base + RTL9300_I2C_MST_DATA_WORD0, &val);
if (ret)
return ret;
data->byte = val & 0xff;
- } else if (size == I2C_SMBUS_WORD_DATA) {
+ break;
+ case I2C_SMBUS_WORD_DATA:
ret = regmap_read(i2c->regmap,
i2c->reg_base + RTL9300_I2C_MST_DATA_WORD0, &val);
if (ret)
return ret;
data->word = val & 0xffff;
- } else {
+ break;
+ case I2C_SMBUS_I2C_BLOCK_DATA:
+ ret = rtl9300_i2c_read(i2c, &data->block[1], len);
+ if (ret)
+ return ret;
+ break;
+ default:
ret = rtl9300_i2c_read(i2c, &data->block[0], len);
if (ret)
return ret;
+ break;
}
}
@@ -296,6 +306,25 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
len = data->block[0] + 1;
break;
+ case I2C_SMBUS_I2C_BLOCK_DATA:
+ ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
+ if (ret)
+ goto out_unlock;
+ if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+ ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
+ if (ret)
+ goto out_unlock;
+ if (read_write == I2C_SMBUS_WRITE) {
+ ret = rtl9300_i2c_write(i2c, &data->block[1], data->block[0]);
+ if (ret)
+ goto out_unlock;
+ }
+ len = data->block[0];
+ break;
+
default:
dev_err(&adap->dev, "Unsupported transaction %d\n", size);
ret = -EOPNOTSUPP;
@@ -314,6 +343,7 @@ static u32 rtl9300_i2c_func(struct i2c_adapter *a)
{
return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
I2C_FUNC_SMBUS_BLOCK_DATA;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer Sven Eckelmann
@ 2025-08-09 17:34 ` Wolfram Sang
0 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2025-08-09 17:34 UTC (permalink / raw)
To: Sven Eckelmann
Cc: Chris Packham, Andi Shyti, linux-i2c, linux-kernel, Jonas Jelonek,
Harshal Gohel, Simon Wunderlich, Alex Guo, stable
[-- Attachment #1: Type: text/plain, Size: 923 bytes --]
On Sat, Aug 09, 2025 at 08:40:54AM +0200, Sven Eckelmann wrote:
> From: Alex Guo <alexguo1023@gmail.com>
>
> The data->block[0] variable comes from user. Without proper check,
> the variable may be very large to cause an out-of-bounds bug.
>
> Fix this bug by checking the value of data->block[0] first.
>
> 1. commit 39244cc75482 ("i2c: ismt: Fix an out-of-bounds bug in
> ismt_access()")
> 2. commit 92fbb6d1296f ("i2c: xgene-slimpro: Fix out-of-bounds bug in
> xgene_slimpro_i2c_xfer()")
>
> Cc: <stable@vger.kernel.org>
> Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
> Signed-off-by: Alex Guo <alexguo1023@gmail.com>
> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write
2025-08-09 6:40 ` [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write Sven Eckelmann
@ 2025-08-09 22:11 ` Jonas Jelonek
2025-08-10 5:59 ` Sven Eckelmann
0 siblings, 1 reply; 9+ messages in thread
From: Jonas Jelonek @ 2025-08-09 22:11 UTC (permalink / raw)
To: Sven Eckelmann, Chris Packham, Andi Shyti
Cc: linux-i2c, linux-kernel, Harshal Gohel, Simon Wunderlich
Hi Sven,
On 09.08.2025 08:40, Sven Eckelmann wrote:
> @@ -314,6 +343,7 @@ static u32 rtl9300_i2c_func(struct i2c_adapter *a)
> {
> return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> + I2C_FUNC_SMBUS_READ_I2C_BLOCK | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
> I2C_FUNC_SMBUS_BLOCK_DATA;
> }
>
Is there a specific reason you explicitly use I2C_FUNC_SMBUS_READ_I2C_BLOCK and
*_WRITE_* instead of I2C_FUNC_SMBUS_I2C_BLOCK ?
Best regards,
Jonas Jelonek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write
2025-08-09 22:11 ` Jonas Jelonek
@ 2025-08-10 5:59 ` Sven Eckelmann
0 siblings, 0 replies; 9+ messages in thread
From: Sven Eckelmann @ 2025-08-10 5:59 UTC (permalink / raw)
To: Chris Packham, Andi Shyti, Jonas Jelonek
Cc: linux-i2c, linux-kernel, Harshal Gohel, Simon Wunderlich
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On Sunday, 10 August 2025 00:11:20 CEST Jonas Jelonek wrote:
> Hi Sven,
>
> On 09.08.2025 08:40, Sven Eckelmann wrote:
> > @@ -314,6 +343,7 @@ static u32 rtl9300_i2c_func(struct i2c_adapter *a)
> > {
> > return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> > I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> > + I2C_FUNC_SMBUS_READ_I2C_BLOCK |
I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
> > I2C_FUNC_SMBUS_BLOCK_DATA;
> > }
> >
>
> Is there a specific reason you explicitly use I2C_FUNC_SMBUS_READ_I2C_BLOCK
and
> *_WRITE_* instead of I2C_FUNC_SMBUS_I2C_BLOCK ?
To be honest, I've just adopted this from the original version of the patch
and didn't spend a second on thinking about a potential simplification.
So yes, thank you for pointing it out. I will integrate it in the patchset and
most likely send out a new version addressing all the comments (until then)
at ~8 pm (GMT+2). The preview can be found at
https://git.open-mesh.org/linux-merge.git/log/?h=b4/i2c-rtl9300-multi-byte
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-08-10 5:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-09 6:40 [PATCH i2c-host-fixes v4 0/5] i2c: rtl9300: Fix multi-byte I2C operations Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 1/5] i2c: rtl9300: Fix out-of-bounds bug in rtl9300_i2c_smbus_xfer Sven Eckelmann
2025-08-09 17:34 ` Wolfram Sang
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 2/5] i2c: rtl9300: Fix multi-byte I2C write Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 3/5] i2c: rtl9300: Increase timeout for transfer polling Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host-fixes v4 4/5] i2c: rtl9300: Add missing count byte for SMBus Block Ops Sven Eckelmann
2025-08-09 6:40 ` [PATCH i2c-host v4 5/5] i2c: rtl9300: Implement I2C block read and write Sven Eckelmann
2025-08-09 22:11 ` Jonas Jelonek
2025-08-10 5:59 ` Sven Eckelmann
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).