| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPnwWgMac8-k729=u6xs03KZ9EEgJeuojqOSE2srTxT8J9YnvA@mail.gmail.com>
Date: Sat, 7 Feb 2026 00:22:19 +0200
From: Michael Zaidman <michael.zaidman@...il.com>
To: Rostislav Nesin <ssp.nesin@...t.ru>
Cc: Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>, linux-i2c@...r.kernel.org,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
lvc-project@...uxtesting.org,
syzbot+64ca69977b37604cd6d9@...kaller.appspotmail.com
Subject: Re: [PATCH] HID: ft260: fix block size validation in SMBus transfers
Hi Rostislav,
Thank you for the patch and for reporting this.
The bug is real: an unvalidated data->block[0] in the native SMBus
path can cause OOB access, and your analysis is correct.
However, I'd prefer to fix this in the I2C core rather than in
individual drivers. Here is why:
drivers/i2c/i2c-core-smbus.c already validates block size in the
emulated path (i2c_smbus_xfer_emulated): for I2C_SMBUS_BLOCK_DATA
write, I2C_SMBUS_I2C_BLOCK_DATA, and I2C_SMBUS_BLOCK_PROC_CALL it
rejects data->block[0] > I2C_SMBUS_BLOCK_MAX with -EINVAL. But when
the adapter provides a native smbus_xfer, __i2c_smbus_xfer() calls
it without any prior validation on data->block[0].
This means every native SMBus adapter is exposed. For example,
hid-mcp2221 has the exact same class of bug -- it uses data->block[0]
in the block paths without any upper-bound check. hid-cp2112 happens
to have a driver-level guard, but relying on each driver to do this
independently is fragile.
Fixing it once in __i2c_smbus_xfer() protects all native SMBus
adapters and aligns the native path with the emulated path's contract.
Also note that the correct bound is data->block[0] > I2C_SMBUS_BLOCK_MAX
(i.e. > 32), consistent with the UAPI definition of union i2c_smbus_data
and the emulated path checks. The + 1 in the submitted patch would allow
33, which is already out of spec per the SMBus standard.
Could you please respin the patch against drivers/i2c/i2c-core-smbus.c
instead? Something like in the attached .diff file, added in
__i2c_smbus_xfer() right
after flags &= ... and before xfer_func = adapter->algo->smbus_xfer
You can keep the same Reported-by, Closes, and Fixes tags. Once this
core patch is in, we won't need separate driver-level checks in
hid-ft260 (or hid-mcp2221).
Thanks,
Michael
On Fri, Feb 6, 2026 at 6:36 AM Rostislav Nesin <ssp.nesin@...t.ru> wrote:
> In ft260_smbus_xfer(), data->block[0] specifies the data length for
> block transfers. Without proper validation, a caller can set block[0]
> to a value larger than I2C_SMBUS_BLOCK_MAX (32), causing out-of-bounds
> access in both ft260_smbus_write() and ft260_i2c_read(). This
> triggered the out-of-bounds access reported by syzbot.
>
> BUG: KASAN: stack-out-of-bounds in ft260_smbus_write+0x19b/0x2f0
> drivers/hid/hid-ft260.c:486
> Read of size 42 at addr ffffc90003427d81 by task syz.2.65/6119
>
> CPU: 0 UID: 0 PID: 6119 Comm: syz.2.65 Not tainted syzkaller #0
> PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 10/25/2025
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:378 [inline]
> print_report+0xcd/0x630 mm/kasan/report.c:482
> kasan_report+0xe0/0x110 mm/kasan/report.c:595
> check_region_inline mm/kasan/generic.c:194 [inline]
> kasan_check_range+0x100/0x1b0 mm/kasan/generic.c:200
> __asan_memcpy+0x23/0x60 mm/kasan/shadow.c:105
> ft260_smbus_write+0x19b/0x2f0 drivers/hid/hid-ft260.c:486
> ft260_smbus_xfer+0x22c/0x640 drivers/hid/hid-ft260.c:736
> __i2c_smbus_xfer drivers/i2c/i2c-core-smbus.c:591 [inline]
> __i2c_smbus_xfer+0x4f0/0xf60 drivers/i2c/i2c-core-smbus.c:554
> i2c_smbus_xfer drivers/i2c/i2c-core-smbus.c:546 [inline]
> i2c_smbus_xfer+0x200/0x3c0 drivers/i2c/i2c-core-smbus.c:536
> i2cdev_ioctl_smbus+0x237/0x990 drivers/i2c/i2c-dev.c:389
> i2cdev_ioctl+0x361/0x840 drivers/i2c/i2c-dev.c:478
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:597 [inline]
> __se_sys_ioctl fs/ioctl.c:583 [inline]
> __x64_sys_ioctl+0x18e/0x210 fs/ioctl.c:583
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xcd/0xf80 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
>
> Add validation for data->block[0] > I2C_SMBUS_BLOCK_MAX + 1 at the
> start of I2C_SMBUS_BLOCK_DATA and I2C_SMBUS_I2C_BLOCK_DATA cases to
> protect both read and write paths.
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
> Reported-by: syzbot+64ca69977b37604cd6d9@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=64ca69977b37604cd6d9
> Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver")
> Signed-off-by: Rostislav Nesin <ssp.nesin@...t.ru>
> ---
> drivers/hid/hid-ft260.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 79505c64dbfe..7bd858e40826 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -659,6 +659,10 @@ static int ft260_smbus_xfer(struct i2c_adapter
> *adapter, u16 addr, u16 flags,
> }
> break;
> case I2C_SMBUS_BLOCK_DATA:
> + if (data->block[0] > I2C_SMBUS_BLOCK_MAX + 1) {
> + ret = -EINVAL;
> + goto smbus_exit;
> + }
> if (read_write == I2C_SMBUS_READ) {
> ret = ft260_smbus_write(dev, addr, cmd, NULL, 0,
> FT260_FLAG_START);
> @@ -675,6 +679,10 @@ static int ft260_smbus_xfer(struct i2c_adapter
> *adapter, u16 addr, u16 flags,
> }
> break;
> case I2C_SMBUS_I2C_BLOCK_DATA:
> + if (data->block[0] > I2C_SMBUS_BLOCK_MAX + 1) {
> + ret = -EINVAL;
> + goto smbus_exit;
> + }
> if (read_write == I2C_SMBUS_READ) {
> ret = ft260_smbus_write(dev, addr, cmd, NULL, 0,
> FT260_FLAG_START);
> --
> 2.34.1
>
Content of type "text/html" skipped
Download attachment "i2c-core-smbus-validate-block-size.diff" of type "application/octet-stream" (1268 bytes)
Powered by blists - more mailing lists