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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Sat, Jun 13, 2026 at 12:39:32AM -0500, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> smb_block_read() takes a destination length but passes it nowhere:
> 
> 	static int smb_block_read(struct rmi_transport_dev *xport,
> 				  u8 commandcode, void *buf, size_t len)
> 	{
> 		...
> 		retval = i2c_smbus_read_block_data(client, commandcode, buf);
> 
> i2c_smbus_read_block_data() has no destination-size argument; it copies
> the block count reported by the device (the first SMBus byte, up to
> I2C_SMBUS_BLOCK_MAX = 32) into buf. The RMI callers pass buffers far
> smaller than 32 bytes - rmi_read_pdt_entry() reads a PDT entry into an
> on-stack u8 buf[RMI_PDT_ENTRY_SIZE] (6 bytes) during the PDT scan - so a
> malfunctioning, malicious or counterfeit RMI4 SMBus controller (or an
> attacker tampering with the I2C bus) that reports a larger block count
> overflows the caller's stack buffer by up to 32 - 6 = 26 bytes,
> clobbering the stack canary, saved registers and the return address.
> 
> Read into a local I2C_SMBUS_BLOCK_MAX-sized buffer and copy back at most
> len bytes, so the device can never write past the caller's buffer.
> 
> Fixes: 82264d0cf7ae ("Input: synaptics-rmi4 - add SMBus support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  drivers/input/rmi4/rmi_smbus.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> index f3d0b40721df..ea957aba28f1 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));

Instead of doing extra copy I'd like to get the following in:

https://lore.kernel.org/all/ZxGrwObOFkNuCn_w@google.com/

But unfortunately it has stalled. Can you try and see if it works for
you?

Thanks.

-- 
Dmitry