Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()

PATCH In review Dmitry Torokhov 2 participants 3 messages Aug 5

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2026-08-05 05:08 UTC #0 lore raw

PATCH 1/1 Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()

When chunking writes into SMBus blocks in rmi_smb_write_block(), the loop calculates block_len using the original total length (len) instead of the remaining length (cur_len).

If len is greater than 32 bytes (SMB_MAX_COUNT), block_len remains 32 for every iteration, even on the final partial chunk where fewer than 32 bytes remain. This causes smb_block_write() to read 32 bytes from the advanced data buffer pointer, reading past the end of the input buffer.

Fix this by calculating block_len using cur_len and advancing the buffer and address pointers by block_len.

Fixes 82264d0cf7aef ("Input: synaptics-rmi4 - add SMBus support")

Cc [email protected]

Reported-by [email protected]

Assisted-by Antigravity:gemini-3.6-flash

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/rmi4/rmi_smbus.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/input/rmi4/rmi_smbus.c +5 -5

index 6de68c602558..d799405fbd33 100644
@@ -140,7 +140,7 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 	u8 commandcode;
 	struct rmi_smb_xport *rmi_smb =
 		container_of(xport, struct rmi_smb_xport, xport);
-	int cur_len = (int)len;
+	size_t cur_len = len;
 
 	mutex_lock(&rmi_smb->page_mutex);

@@ -148,7 +148,7 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 		/*
 		 * break into 32 bytes chunks to write get command code
 		 */
-		int block_len = min_t(int, len, SMB_MAX_COUNT);
+		int block_len = min_t(size_t, cur_len, SMB_MAX_COUNT);
 
 		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
 						  false, &commandcode);

@@ -161,9 +161,9 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 			goto exit;
 
 		/* prepare to write next block of bytes */
-		cur_len -= SMB_MAX_COUNT;
-		databuff += SMB_MAX_COUNT;
-		rmiaddr += SMB_MAX_COUNT;
+		cur_len -= block_len;
+		databuff += block_len;
+		rmiaddr += block_len;
 	}
 exit:
 	mutex_unlock(&rmi_smb->page_mutex);

Signature · 5 lines

2.55.0.571.g244d577d93-goog


-- 
Dmitry

[email protected]

AI review

2026-08-05 05:20 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing stack/heap buffer overflow in rmi_smb_read_block() due to unsafe use of i2c_smbus_read_block_data() without a bounce buffer. --

--- Patch [1]: [PATCH] Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block() --- commit 530e4667b6144c7e030412bbc818f1de86a5a09e
Author: Dmitry Torokhov <[email protected]>

Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()

This commit fixes an out-of-bounds read in rmi_smb_write_block() by ensuring block_len is calculated using the remaining length (cur_len) rather than the original total length.

Quoted 3 lines

drivers/input/rmi4/rmi_smbus.c +0 -0

[ ... ]

drivers/input/rmi4/rmi_smbus.c static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr, · line 154

Quoted 9 lines

drivers/input/rmi4/rmi_smbus.c +1 -1

@@ -148,7 +148,7 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 		/*
 		 * break into 32 bytes chunks to write get command code
 		 */
-		int block_len = min_t(int, len, SMB_MAX_COUNT);
+		int block_len = min_t(size_t, cur_len, SMB_MAX_COUNT);
 
 		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
 						  false, &commandcode);

[ ... ]

[Severity: High]
This is a pre-existing issue, but since the chunking logic in rmi_smb_write_block() is being corrected here, does a similar buffer overflow exist in the read path?

Looking at smb_block_read(), it ignores the len argument and passes the buffer directly to i2c_smbus_read_block_data():

drivers/input/rmi4/rmi_smbus.c:smb_block_read() {
    ...
    retval = i2c_smbus_read_block_data(client, commandcode, buf);
    ...
}

If a caller requests a small read, or if the final partial chunk in rmi_smb_read_block() has less than 32 bytes of capacity remaining:

drivers/input/rmi4/rmi_smbus.c:rmi_smb_read_block() {
    ...
    retval = smb_block_read(xport, commandcode, databuff, block_len);
    ...
}

Could a malfunctioning I2C device returning a length greater than block_len cause the I2C core to unconditionally copy up to 32 bytes, overflowing the databuff allocation?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Dmitry Torokhov

author maintainer

2026-08-05 05:23 UTC #2 lore raw

Quoted 54 lines · On Wed, Aug 05, 2026 at 05:20:39AM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Pre-existing stack/heap buffer overflow in rmi_smb_read_block() due to unsafe use of i2c_smbus_read_block_data() without a bounce buffer. --
--- Patch [1]: [PATCH] Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block() --- commit 530e4667b6144c7e030412bbc818f1de86a5a09e
Author: Dmitry Torokhov <[email protected]>
Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()
This commit fixes an out-of-bounds read in rmi_smb_write_block() by ensuring block_len is calculated using the remaining length (cur_len) rather than the original total length.
Quoted 3 lines
drivers/input/rmi4/rmi_smbus.c +0 -0
[ ... ]
drivers/input/rmi4/rmi_smbus.c static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr, · line 154
Quoted 9 lines
drivers/input/rmi4/rmi_smbus.c +1 -1
@@ -148,7 +148,7 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 		/*
 		 * break into 32 bytes chunks to write get command code
 		 */
-		int block_len = min_t(int, len, SMB_MAX_COUNT);
+		int block_len = min_t(size_t, cur_len, SMB_MAX_COUNT);
 
 		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
 						  false, &commandcode);
[ ... ]
[Severity: High]
This is a pre-existing issue, but since the chunking logic in rmi_smb_write_block() is being corrected here, does a similar buffer overflow exist in the read path?
Looking at smb_block_read(), it ignores the len argument and passes the buffer directly to i2c_smbus_read_block_data():
drivers/input/rmi4/rmi_smbus.c:smb_block_read() {
    ...
    retval = i2c_smbus_read_block_data(client, commandcode, buf);
    ...
}
If a caller requests a small read, or if the final partial chunk in rmi_smb_read_block() has less than 32 bytes of capacity remaining:
drivers/input/rmi4/rmi_smbus.c:rmi_smb_read_block() {
    ...
    retval = smb_block_read(xport, commandcode, databuff, block_len);
    ...
}

Quoted 4 lines

Could a malfunctioning I2C device returning a length greater than block_len cause the I2C core to unconditionally copy up to 32 bytes, overflowing the databuff allocation?

Yes, but I hope this can be addressed once improved version of i2c_smbus_read_block_data() lands.

Thanks.

Signature · 1 line

Dmitry