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

[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