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

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

[Bryam Vargas via B4 Relay]

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));
 	return retval;
 }
 

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260613-b4-disp-2e033955-ffe3889ce3e9

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>

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

[sashiko-bot]

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