[PATCH 0/2] Input: clamp SMBus block reads to the caller's buffer

[PATCH 0/2] Input: clamp SMBus block reads to the caller's buffer

[Chengyu Liu]

i2c_smbus_read_block_data() has no length parameter: it copies the
device-reported byte count (clamped to I2C_SMBUS_BLOCK_MAX by the
core) into the caller's buffer, so callers must pass a full-size
buffer.  An audit of all call sites found two input drivers passing
smaller stack buffers:

 - rmi4: rmi_read_pdt_entry() reads PDT entries into a 6-byte stack
   buffer during PDT scanning at probe time; a device reporting a
   count of 32 overruns it by 26 bytes.

 - cyapa: cyapa_gen3_irq_handler() reads into a 27-byte struct
   cyapa_reg_data on every touch interrupt; a count of 32 overruns it
   by 5 bytes, in interrupt context.

In both cases the contents are controlled by the device.  Bounce the
reads through a full-size buffer and clamp the copy to the requested
length.

Found by static analysis (audit of i2c_smbus_read_block_data()
callers).

Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---

Chengyu Liu (2):
  Input: rmi4 - clamp SMBus block reads to the requested length
  Input: cyapa - clamp SMBus block reads to the caller's buffer

 drivers/input/mouse/cyapa_gen3.c | 14 ++++++++++++--
 drivers/input/rmi4/rmi_smbus.c   | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/2] Input: rmi4 - clamp SMBus block reads to the requested length

[Chengyu Liu]

smb_block_read() drops its len argument and calls
i2c_smbus_read_block_data(), which copies the device-reported byte
count into the caller's buffer.  The core clamps the count to
I2C_SMBUS_BLOCK_MAX (32), but nothing bounds it to the caller's buffer.

rmi_read_pdt_entry() passes a 6-byte stack buffer during PDT scanning
at probe time: a device reporting a count of 32 overruns it by 26
bytes, with contents controlled by the device.  rmi_read() with len=1
and all register reads with len < 32 have the same problem.

Bounce the read through a full-size I2C_SMBUS_BLOCK_MAX buffer and
clamp the copy to the requested length.

Found by static analysis (audit of i2c_smbus_read_block_data()
callers).

Fixes: 82264d0cf7ae ("Input: synaptics-rmi4 - add SMBus support")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
 drivers/input/rmi4/rmi_smbus.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 8b61c23822..e4d77d1f80 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -177,8 +177,23 @@ 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 tmp[I2C_SMBUS_BLOCK_MAX];
+	int retval;
+
+	/*
+	 * i2c_smbus_read_block_data() copies the device-reported count
+	 * (up to I2C_SMBUS_BLOCK_MAX bytes) without knowing the caller's
+	 * buffer size; bounce through a full-size buffer to bound the copy.
+	 */
+	retval = i2c_smbus_read_block_data(client, commandcode, tmp);
+	if (retval < 0)
+		return retval;
 
-	return i2c_smbus_read_block_data(client, commandcode, buf);
+	if ((size_t)retval > len)
+		retval = len;
+	memcpy(buf, tmp, retval);
+
+	return retval;
 }
 
 static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
-- 
2.25.1

[PATCH 2/2] Input: cyapa - clamp SMBus block reads to the caller's buffer

[Chengyu Liu]

cyapa_smbus_read_block() hands the caller's buffer straight to
i2c_smbus_read_block_data() without passing the buffer size; the i2c
core then copies the device-provided count, and any count <= 32 passes
the adapters' range checks.  Both the group-read branch and the indexed
loop (on its final partial chunk) can therefore write past the caller's
buffer.

The affected callers use 27-byte stack objects:
cyapa_gen3_irq_handler() (struct cyapa_reg_data, on every touch
interrupt) and cyapa_gen3_get_query_data() (at probe).  A device
reporting a count of 32 overruns them by 5 bytes, with contents
controlled by the device, in interrupt context.

Bounce through a full-size I2C_SMBUS_BLOCK_MAX buffer and clamp the
copy to the requested length in both branches.

Found by static analysis.

Fixes: 9f1cd857518f ("Input: cyapa - re-design driver to support multi-trackpad in one driver")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
 drivers/input/mouse/cyapa_gen3.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index fc3fb95452..cd80ae51f4 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -254,20 +254,30 @@ ssize_t cyapa_smbus_read_block(struct cyapa *cyapa, u8 cmd, size_t len,
 		return -EINVAL;
 
 	if (SMBUS_GROUP_BLOCK_CMD_MASK & cmd) {
+		u8 tmp[I2C_SMBUS_BLOCK_MAX];
+
 		/* read specific block registers command. */
 		smbus_cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ);
-		ret = i2c_smbus_read_block_data(client, smbus_cmd, values);
+		ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
+		if (ret > 0)
+			/* never trust the device-provided count */
+			memcpy(values, tmp, min_t(size_t, ret, len));
 		goto out;
 	}
 
 	ret = 0;
 	for (index = 0; index * I2C_SMBUS_BLOCK_MAX < len; index++) {
+		u8 tmp[I2C_SMBUS_BLOCK_MAX];
+		size_t chunk = min_t(size_t, len - index * I2C_SMBUS_BLOCK_MAX,
+				     I2C_SMBUS_BLOCK_MAX);
+
 		smbus_cmd = SMBUS_ENCODE_IDX(cmd, index);
 		smbus_cmd = SMBUS_ENCODE_RW(smbus_cmd, SMBUS_READ);
 		buf = values + I2C_SMBUS_BLOCK_MAX * index;
-		ret = i2c_smbus_read_block_data(client, smbus_cmd, buf);
+		ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
 		if (ret < 0)
 			goto out;
+		memcpy(buf, tmp, min_t(size_t, ret, chunk));
 	}
 
 out:
-- 
2.25.1

Re: [PATCH 2/2] Input: cyapa - clamp SMBus block reads to the caller's buffer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Input: cyapa - cyapa_smbus_read_block() returns full len on short block reads
--

commit 17f2cef41b86752525f835330272cc728242a63c
Author: Chengyu Liu <lui2012qwq@gmail.com>

Input: cyapa - clamp SMBus block reads to the caller's buffer

This commit bounces SMBus block reads in cyapa_smbus_read_block() through
a temporary I2C_SMBUS_BLOCK_MAX buffer and clamps the copy size to the
caller's requested length to prevent buffer overruns.

> diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
> index fc3fb954523b7..cd80ae51f4e52 100644
> --- a/drivers/input/mouse/cyapa_gen3.c
> +++ b/drivers/input/mouse/cyapa_gen3.c
> @@ -254,20 +254,30 @@ ssize_t cyapa_smbus_read_block(struct cyapa *cyapa, u8 cmd, size_t len,
>  		return -EINVAL;
>  
>  	if (SMBUS_GROUP_BLOCK_CMD_MASK & cmd) {
> +		u8 tmp[I2C_SMBUS_BLOCK_MAX];
> +
>  		/* read specific block registers command. */
>  		smbus_cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ);
> -		ret = i2c_smbus_read_block_data(client, smbus_cmd, values);
> +		ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
> +		if (ret > 0)
> +			/* never trust the device-provided count */
> +			memcpy(values, tmp, min_t(size_t, ret, len));
>  		goto out;
>  	}
>  
>  	ret = 0;
>  	for (index = 0; index * I2C_SMBUS_BLOCK_MAX < len; index++) {
> +		u8 tmp[I2C_SMBUS_BLOCK_MAX];
> +		size_t chunk = min_t(size_t, len - index * I2C_SMBUS_BLOCK_MAX,
> +				     I2C_SMBUS_BLOCK_MAX);
> +
>  		smbus_cmd = SMBUS_ENCODE_IDX(cmd, index);
>  		smbus_cmd = SMBUS_ENCODE_RW(smbus_cmd, SMBUS_READ);
>  		buf = values + I2C_SMBUS_BLOCK_MAX * index;
> -		ret = i2c_smbus_read_block_data(client, smbus_cmd, buf);
> +		ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
>  		if (ret < 0)
>  			goto out;
> +		memcpy(buf, tmp, min_t(size_t, ret, chunk));
>  	}

[Severity: High]
What happens in cyapa_smbus_read_block() if the device returns a short block
count where 0 < ret < len (or 0 <= ret < chunk in the indexed loop)?

Both branches copy only ret bytes into values or buf, leaving the remainder
of the caller's buffer untouched, and then jump to out:

drivers/input/mouse/cyapa_gen3.c:cyapa_smbus_read_block() {
	...
out:
	return ret > 0 ? len : ret;
}

Because cyapa_smbus_read_block() still returns the full requested len whenever
the last i2c_smbus_read_block_data() call returned a positive count, callers
that pass uninitialized stack buffers and check for a full read cannot detect
the short read:

drivers/input/mouse/cyapa_gen3.c:cyapa_gen3_get_query_data() {
	u8 query_data[QUERY_DATA_SIZE];
	int ret;

	/* query dataabda has 27 bytes */
	ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_QUERY, query_data);
	if (ret != QUERY_DATA_SIZE)
		return (ret < 0) ? ret : -EIO;
	...
}

drivers/input/mouse/cyapa_gen3.c:cyapa_gen3_irq_handler() {
	struct cyapa_reg_data data;
	int ret;

	ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_DATA, (u8 *)&data);
	if (ret != sizeof(data)) {
		dev_err(dev, "failed to read report data, (%d)\n", ret);
		return -EINVAL;
	}
	...
}

Wouldn't a device returning fewer bytes than requested cause
cyapa_gen3_get_query_data(), cyapa_gen3_irq_handler(),
cyapa_gen3_try_poll_handler(), and cyapa_get_state() to consume uninitialized
stack memory in query_data, data, and status (and expose uninitialized bytes
from query_data via the product_id and firmware_version sysfs attributes)?
Should cyapa_smbus_read_block() return an error such as -EIO when ret < len
in the group-read branch or ret < chunk in the indexed loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103131127.1585884-1-lui2012qwq@gmail.com?part=2

Re: [PATCH 2/2] Input: cyapa - clamp SMBus block reads to the caller's buffer

[cy l]

Thanks, correct. The masking predates the clamp (the out: "ret > 0 ?
len : ret" convention), but it sits in the same length semantics, so
v2 handles it: both branches now fail short reads with -EIO (group
branch: ret < len; indexed loop: ret < chunk). rmi4 (patch 1/2) is
unaffected: rmi_smb_read_block() zeroes the caller's buffer upfront,
so a short read leaves zeros, not stack garbage.

On Tue, Nov 3, 2026 at 9:22 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Input: cyapa - cyapa_smbus_read_block() returns full len on short block reads
> --
>
> commit 17f2cef41b86752525f835330272cc728242a63c
> Author: Chengyu Liu <lui2012qwq@gmail.com>
>
> Input: cyapa - clamp SMBus block reads to the caller's buffer
>
> This commit bounces SMBus block reads in cyapa_smbus_read_block() through
> a temporary I2C_SMBUS_BLOCK_MAX buffer and clamps the copy size to the
> caller's requested length to prevent buffer overruns.
>
> > diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
> > index fc3fb954523b7..cd80ae51f4e52 100644
> > --- a/drivers/input/mouse/cyapa_gen3.c
> > +++ b/drivers/input/mouse/cyapa_gen3.c
> > @@ -254,20 +254,30 @@ ssize_t cyapa_smbus_read_block(struct cyapa *cyapa, u8 cmd, size_t len,
> >               return -EINVAL;
> >
> >       if (SMBUS_GROUP_BLOCK_CMD_MASK & cmd) {
> > +             u8 tmp[I2C_SMBUS_BLOCK_MAX];
> > +
> >               /* read specific block registers command. */
> >               smbus_cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ);
> > -             ret = i2c_smbus_read_block_data(client, smbus_cmd, values);
> > +             ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
> > +             if (ret > 0)
> > +                     /* never trust the device-provided count */
> > +                     memcpy(values, tmp, min_t(size_t, ret, len));
> >               goto out;
> >       }
> >
> >       ret = 0;
> >       for (index = 0; index * I2C_SMBUS_BLOCK_MAX < len; index++) {
> > +             u8 tmp[I2C_SMBUS_BLOCK_MAX];
> > +             size_t chunk = min_t(size_t, len - index * I2C_SMBUS_BLOCK_MAX,
> > +                                  I2C_SMBUS_BLOCK_MAX);
> > +
> >               smbus_cmd = SMBUS_ENCODE_IDX(cmd, index);
> >               smbus_cmd = SMBUS_ENCODE_RW(smbus_cmd, SMBUS_READ);
> >               buf = values + I2C_SMBUS_BLOCK_MAX * index;
> > -             ret = i2c_smbus_read_block_data(client, smbus_cmd, buf);
> > +             ret = i2c_smbus_read_block_data(client, smbus_cmd, tmp);
> >               if (ret < 0)
> >                       goto out;
> > +             memcpy(buf, tmp, min_t(size_t, ret, chunk));
> >       }
>
> [Severity: High]
> What happens in cyapa_smbus_read_block() if the device returns a short block
> count where 0 < ret < len (or 0 <= ret < chunk in the indexed loop)?
>
> Both branches copy only ret bytes into values or buf, leaving the remainder
> of the caller's buffer untouched, and then jump to out:
>
> drivers/input/mouse/cyapa_gen3.c:cyapa_smbus_read_block() {
>         ...
> out:
>         return ret > 0 ? len : ret;
> }
>
> Because cyapa_smbus_read_block() still returns the full requested len whenever
> the last i2c_smbus_read_block_data() call returned a positive count, callers
> that pass uninitialized stack buffers and check for a full read cannot detect
> the short read:
>
> drivers/input/mouse/cyapa_gen3.c:cyapa_gen3_get_query_data() {
>         u8 query_data[QUERY_DATA_SIZE];
>         int ret;
>
>         /* query dataabda has 27 bytes */
>         ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_QUERY, query_data);
>         if (ret != QUERY_DATA_SIZE)
>                 return (ret < 0) ? ret : -EIO;
>         ...
> }
>
> drivers/input/mouse/cyapa_gen3.c:cyapa_gen3_irq_handler() {
>         struct cyapa_reg_data data;
>         int ret;
>
>         ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_DATA, (u8 *)&data);
>         if (ret != sizeof(data)) {
>                 dev_err(dev, "failed to read report data, (%d)\n", ret);
>                 return -EINVAL;
>         }
>         ...
> }
>
> Wouldn't a device returning fewer bytes than requested cause
> cyapa_gen3_get_query_data(), cyapa_gen3_irq_handler(),
> cyapa_gen3_try_poll_handler(), and cyapa_get_state() to consume uninitialized
> stack memory in query_data, data, and status (and expose uninitialized bytes
> from query_data via the product_id and firmware_version sysfs attributes)?
> Should cyapa_smbus_read_block() return an error such as -EIO when ret < len
> in the group-read branch or ret < chunk in the indexed loop?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261103131127.1585884-1-lui2012qwq@gmail.com?part=2

Re: [PATCH 0/2] Input: clamp SMBus block reads to the caller's buffer

[Dmitry Torokhov]

Hi Chengyu,

On Tue, Nov 03, 2026 at 09:11:25PM +0800, Chengyu Liu wrote:
> i2c_smbus_read_block_data() has no length parameter: it copies the
> device-reported byte count (clamped to I2C_SMBUS_BLOCK_MAX by the
> core) into the caller's buffer, so callers must pass a full-size
> buffer.  An audit of all call sites found two input drivers passing
> smaller stack buffers:
> 
>  - rmi4: rmi_read_pdt_entry() reads PDT entries into a 6-byte stack
>    buffer during PDT scanning at probe time; a device reporting a
>    count of 32 overruns it by 26 bytes.
> 
>  - cyapa: cyapa_gen3_irq_handler() reads into a 27-byte struct
>    cyapa_reg_data on every touch interrupt; a count of 32 overruns it
>    by 5 bytes, in interrupt context.
> 
> In both cases the contents are controlled by the device.  Bounce the
> reads through a full-size buffer and clamp the copy to the requested
> length.

No, we should be using the new 4-argument variant of 
i2c_smbus_read_block_data() that I posted earlier.

Thanks.

-- 
Dmitry