[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

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

[Dmitry Torokhov]

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

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

[Bryam Vargas]

On 2026-06-23, Dmitry Torokhov wrote:
> Wolfram, any chance we could get this in? I am getting patches for OOB

Andi, looping you in -- MAINTAINERS hands I2C to you as of v7.1, and this is
Dmitry's 2024 i2c-core safety patch, which I just retested against current
mainline.

Dmitry, it works. You asked me to try it on the rmi_smbus stack overflow I
reported. On current mainline (master 502d801f0ab0) the patch applies with no
change after ~20 months. i2c-core-smbus.o builds, and so do the unchanged 3-arg
callers (ipmi_ssif, pmbus_core) and the 4-arg form. Nothing takes the address
of i2c_smbus_read_block_data(), so the transition macro compiles for every
caller -- each of the ~82 is a plain 3- or 4-arg call.

The concrete user that was missing in 2024: rmi_read_pdt_entry() reads a PDT
entry into an on-stack u8 buf[6], and smb_block_read() hands that buffer to
i2c_smbus_read_block_data() with no size, so a device reporting a block count
of 7..32 smashes the stack. The 4-arg form bounds it to 6:

    -	retval = i2c_smbus_read_block_data(client, commandcode, buf);
    +	retval = i2c_smbus_read_block_data(client, commandcode, len, buf);

Why 4-arg specifically: the 3-arg default (length = I2C_SMBUS_BLOCK_MAX) is
byte-identical to today's code, so the core patch alone doesn't fix an
undersized caller -- the safety comes from moving each such caller to 4-arg,
which the rmi_smbus conversion above demonstrates.

rmi_smbus is the undersized caller here; the other block-data callers I checked
size to I2C_SMBUS_BLOCK_MAX, so this is the one live conversion for now and the
macro mainly guards the next small buffer.

I build-tested the above on mainline and A/B-verified the bound with a
userspace ASan mirror (-m64 and -m32; the unbounded value is device payload,
not bus timing, so no RMI hardware is needed): 3-arg/today faults for a count
of 7..32 into buf[6], 4-arg is clean for every count. I didn't boot-test a
patched kernel, so I'm not sending a Tested-by tag -- but happy to add one if
you want a specific config exercised.

I can post the rmi_smbus conversion as a formal patch on top of yours (it's
ready, checkpatch-clean), or you can fold it -- whichever you prefer.

Thanks,
Bryam