[PATCH v2] i2c: dev: cap msg length against allocated buffer in i2cdev_ioctl_rdwr

[PATCH v2] i2c: dev: cap msg length against allocated buffer in i2cdev_ioctl_rdwr

[Xinhao Lv]

From: Xinhao Lv <lvxinhao3@xiaomi.com>

In i2cdev_ioctl_rdwr(), the buffer for each I2C message is allocated
via memdup_user() using the user-supplied msgs[i].len. However, the
underlying I2C adapter driver may modify msgs[i].len during
i2c_transfer() (e.g., for I2C_M_RECV_LEN messages where the actual
length is determined by the slave device).

After i2c_transfer() returns, copy_to_user() uses the potentially
modified msgs[i].len without verifying it still fits within the
originally allocated buffer. If an adapter driver sets msgs[i].len to a
value larger than the allocation, this results in an out-of-bounds read
from the SLUB object, which CONFIG_HARDENED_USERCOPY detects as a
kernel memory exposure attempt:

  usercopy: Kernel memory exposure attempt detected from SLUB object
  'kmalloc-128' (offset 0, size 271)!

  Call trace:
   usercopy_abort+0xe0/0xe4
   __check_heap_object+0xd0/0xf0
   __check_object_size+0x398/0x4a0
   i2cdev_ioctl_rdwr+0x298/0x3f0
   i2cdev_ioctl+0x160/0x5a0
   __arm64_sys_ioctl+0x114/0x170

Fix this by saving each message's original allocation length before
calling i2c_transfer(), and capping msgs[i].len to that saved length
before copy_to_user(). This ensures we never copy more bytes than were
actually allocated, regardless of what the adapter driver does to the
length field.

Found by syzkaller on an ARM64 platform.

Signed-off-by: Xinhao Lv <lvxinhao3@xiaomi.com>
---
v1 -> v2: resend with git-send-email to fix formatting (v1 was mangled
by Outlook, leading whitespace in diff was stripped)

 drivers/i2c/i2c-dev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index ccaac5e..af5b0a7 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -244,6 +244,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		unsigned nmsgs, struct i2c_msg *msgs)
 {
 	u8 __user **data_ptrs;
+	u16 *orig_lens;
 	int i, res;
 
 	/* Adapter must support I2C transfers */
@@ -254,6 +255,12 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 	if (!data_ptrs)
 		return -ENOMEM;
 
+	orig_lens = kmalloc_array(nmsgs, sizeof(u16), GFP_KERNEL);
+	if (!orig_lens) {
+		kfree(data_ptrs);
+		return -ENOMEM;
+	}
+
 	res = 0;
 	for (i = 0; i < nmsgs; i++) {
 		/* Limit the size of the message to a sane amount */
@@ -268,6 +275,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 			res = PTR_ERR(msgs[i].buf);
 			break;
 		}
+		orig_lens[i] = msgs[i].len;
 		/* memdup_user allocates with GFP_KERNEL, so DMA is ok */
 		msgs[i].flags |= I2C_M_DMA_SAFE;
 
@@ -300,12 +308,15 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		for (j = 0; j < i; ++j)
 			kfree(msgs[j].buf);
 		kfree(data_ptrs);
+		kfree(orig_lens);
 		return res;
 	}
 
 	res = i2c_transfer(client->adapter, msgs, nmsgs);
 	while (i-- > 0) {
 		if (res >= 0 && (msgs[i].flags & I2C_M_RD)) {
+			if (msgs[i].len > orig_lens[i])
+				msgs[i].len = orig_lens[i];
 			if (copy_to_user(data_ptrs[i], msgs[i].buf,
 					 msgs[i].len))
 				res = -EFAULT;
@@ -313,6 +324,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		kfree(msgs[i].buf);
 	}
 	kfree(data_ptrs);
+	kfree(orig_lens);
 	return res;
 }
 
-- 
2.50.1