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

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.
> 
> > 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