Re: [PATCH] Input: synaptics-rmi4 - bound the SMBus block read to the caller buffer

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds read and information leak in `rmi_smb_write_block`.
--

commit d7c26cbb5c7a78f15621da67eb9707ffe6750b59
Author: Bryam Vargas <hexlabsecurity@proton.me>

Input: synaptics-rmi4 - bound the SMBus block read to the caller buffer

This commit fixes a stack overflow in smb_block_read by reading into a local
buffer first. It then copies back at most len bytes to ensure the device
never writes past the caller's buffer.

> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> index f3d0b40721df0..ea957aba28f17 100644
> --- a/drivers/input/rmi4/rmi_smbus.c
> +++ b/drivers/input/rmi4/rmi_smbus.c
> @@ -177,12 +177,20 @@ static int smb_block_read(struct rmi_transport_dev *xport,
>  	struct rmi_smb_xport *rmi_smb =
>  		container_of(xport, struct rmi_smb_xport, xport);
>  	struct i2c_client *client = rmi_smb->client;
> +	u8 data[I2C_SMBUS_BLOCK_MAX];
>  	int retval;
>  
> -	retval = i2c_smbus_read_block_data(client, commandcode, buf);
> +	/*
> +	 * i2c_smbus_read_block_data() copies the device-reported block count
> +	 * (up to I2C_SMBUS_BLOCK_MAX) into the destination and has no way to
> +	 * know its size, so read into a local buffer and copy back at most
> +	 * len bytes - never past the caller's buffer.
> +	 */
> +	retval = i2c_smbus_read_block_data(client, commandcode, data);
>  	if (retval < 0)
>  		return retval;
>  
> +	memcpy(buf, data, min_t(size_t, retval, len));
>  	return retval;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while looking at this file,
I noticed a similar size calculation issue in rmi_smb_write_block().

Does this code overflow databuff on the final chunk if len is not a
multiple of 32?

drivers/input/rmi4/rmi_smbus.c:rmi_smb_write_block() {
    ...
	while (cur_len > 0) {
		/*
		 * break into 32 bytes chunks to write get command code
		 */
		int block_len = min_t(int, len, SMB_MAX_COUNT);
    ...
		retval = smb_block_write(xport, commandcode,
					 databuff, block_len);
    ...
		cur_len -= SMB_MAX_COUNT;
		databuff += SMB_MAX_COUNT;
	}
    ...
}

It appears block_len is calculated using the invariant len parameter instead
of cur_len. If len is greater than 32 but not a multiple of 32 (e.g., 40),
the second iteration will evaluate min(40, 32) = 32 instead of the
remaining 8 bytes.

This would cause smb_block_write() to read 32 bytes from databuff when
only 8 valid bytes remain, leading to an out-of-bounds read.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-2e033955-v1-1-43ab7281667a@proton.me?part=1