Re: [PATCH v3] i2c: smbus: reject oversized block transfers in the common path

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On Wed, May 06, 2026 at 01:55:11AM +0800, Weiming Shi wrote:
> The SMBus block transfer length data->block[0] is validated in
> i2c_smbus_xfer_emulated() but that check runs too late for tracepoints
> and is skipped entirely when the adapter provides a native smbus_xfer
> implementation. This allows user-controlled oversized block lengths to
> reach tracepoint memcpy calls and driver callbacks unchecked.
> 
> Add an early validation in __i2c_smbus_xfer() that rejects block
> transfers whose caller-supplied length is zero or exceeds
> I2C_SMBUS_BLOCK_MAX before any tracepoint fires or driver callback
> runs. data->block[0] is filled in by the device on SMBus block reads,
> so the check is scoped to operations where the length is actually
> supplied by the caller. This is consistent with the existing -EINVAL
> convention in the emulated path and protects all downstream consumers
> at once: the smbus_write tracepoint, all native smbus_xfer driver
> implementations, and the emulated path.
> 
> Two distinct bugs are fixed by this change:
> 
> Bug 1: smbus_write tracepoint OOB (include/trace/events/smbus.h)
>   trace_smbus_write() fires before any validation and copies
>   data->block[0]+1 bytes into a 34-byte event buffer. With
>   block[0]=0xfe the tracepoint copies 255 bytes, overflowing by 221.
> 
>  BUG: KASAN: stack-out-of-bounds in trace_event_raw_event_smbus_write+0x27c/0x530
>  Read of size 255 at addr ffff88800d98fcf8 by task poc_smbus/91
>  Call Trace:
>   <TASK>
>   __asan_memcpy+0x23/0x80
>   trace_event_raw_event_smbus_write+0x27c/0x530
>   __i2c_smbus_xfer+0x43a/0xa40
>   i2c_smbus_xfer+0x19e/0x340
>   i2cdev_ioctl_smbus+0x38f/0x7f0
>   i2cdev_ioctl+0x35e/0x680
>   __x64_sys_ioctl+0x147/0x1e0
>   do_syscall_64+0xcf/0x15a0
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   </TASK>
> 
> Bug 2: i2c-stub I2C_SMBUS_I2C_BLOCK_DATA OOB (drivers/i2c/i2c-stub.c)
>   stub_xfer() implements .smbus_xfer directly and only clamps
>   block[0] against 256-command, not I2C_SMBUS_BLOCK_MAX. With
>   block[0]=0xff and command=0 the loop accesses block[1+i] for
>   i up to 254, far past the 34-byte union.
> 
>  UBSAN: array-index-out-of-bounds in drivers/i2c/i2c-stub.c:223:44
>  index 34 is out of range for type '__u8 [34]'
>  Call Trace:
>   <TASK>
>   __ubsan_handle_out_of_bounds+0xd7/0x120
>   stub_xfer+0x1971/0x198f [i2c_stub]
>   __i2c_smbus_xfer+0x306/0xa40
>   i2c_smbus_xfer+0x19e/0x340
>   i2cdev_ioctl_smbus+0x38f/0x7f0
>   i2cdev_ioctl+0x35e/0x680
>   __x64_sys_ioctl+0x147/0x1e0
>   do_syscall_64+0xcf/0x15a0
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   </TASK>
> 
> Both traces reproduced on v7.0-rc6+i2c/for-current with KASAN+UBSAN.
> 
> Fixes: 8a325997d95d ("i2c: Add message transfer tracepoints for SMBUS [ver #2]")
> Fixes: 4710317891e4 ("i2c-stub: Implement I2C block support")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]