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

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

[Dmitry Torokhov]

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: stable@vger.kernel.org
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_smbus.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 6de68c602558..d799405fbd33 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -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);
-- 
2.55.0.571.g244d577d93-goog


-- 
Dmitry

Re: [PATCH] Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()

[sashiko-bot]

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 <dmitry.torokhov@gmail.com>

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.

> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> --- a/drivers/input/rmi4/rmi_smbus.c
> +++ b/drivers/input/rmi4/rmi_smbus.c
[ ... ]
> @@ -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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anLFSMKSoKyyZ272@google.com?part=1

Re: [PATCH] Input: rmi_smbus - fix out-of-bounds read in rmi_smb_write_block()

[Dmitry Torokhov]

On Wed, Aug 05, 2026 at 05:20:39AM +0000, sashiko-bot@kernel.org 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 <dmitry.torokhov@gmail.com>
> 
> 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.
> 
> > diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> > --- a/drivers/input/rmi4/rmi_smbus.c
> > +++ b/drivers/input/rmi4/rmi_smbus.c
> [ ... ]
> > @@ -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?
> 

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

Thanks.

-- 
Dmitry