i2c: stub: Reject I2C block transfers with invalid length
Subsystems (3)
PATCH v2 1/1 i2c: stub: Reject I2C block transfers with invalid length
The I2C_SMBUS_I2C_BLOCK_DATA case in stub_xfer() uses data->block[0] as the transfer length. The existing check only clamps it to avoid overrunning the chip->words[256] register array, but does not validate it against I2C_SMBUS_BLOCK_MAX (32), which is the limit of the union i2c_smbus_data.block buffer (34 bytes total). The driver is a development/test tool (CONFIG_I2C_STUB=m, not built by default) that must be loaded with a chip_addr= parameter.
A local user with access to /dev/i2c-* can issue an I2C_SMBUS ioctl with I2C_SMBUS_I2C_BLOCK_DATA and data->block[0] > 32, causing stub_xfer() to read or write past the end of the union i2c_smbus_data.block buffer:
BUG: KASAN: stack-out-of-bounds in stub_xfer (drivers/i2c/i2c-stub.c:223) Read of size 1 at addr ffff88800abcfd92 by task exploit/81 Call Trace: <TASK> stub_xfer (drivers/i2c/i2c-stub.c:223) __i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:593) i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:536) i2cdev_ioctl_smbus (drivers/i2c/i2c-dev.c:391) i2cdev_ioctl (drivers/i2c/i2c-dev.c:478) __x64_sys_ioctl (fs/ioctl.c:583) do_syscall_64 (arch/x86/entry/syscall_64.c:94) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) </TASK>
The bug exists because i2c-stub implements .smbus_xfer directly, bypassing the I2C_SMBUS_BLOCK_MAX validation in i2c_smbus_xfer_emulated(). The I2C_SMBUS_BLOCK_DATA case in the same function correctly validates against I2C_SMBUS_BLOCK_MAX, but the I2C_SMBUS_I2C_BLOCK_DATA case does not.
Fix by rejecting transfers with data->block[0] == 0 or data->block[0] > I2C_SMBUS_BLOCK_MAX with -EINVAL, consistent with both the I2C_SMBUS_BLOCK_DATA case in the same function and the I2C_SMBUS_I2C_BLOCK_DATA validation in i2c_smbus_xfer_emulated().
---
v2:
- Also reject data->block[0] == 0 (suggested by Jean Delvare).
drivers/i2c/i2c-stub.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/i2c/i2c-stub.c +5 -0
@@ -214,6 +214,11 @@ static s32 stub_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags,* We ignore banks here, because banked chips don't use I2C * block transfers */ +if (data->block[0 ] ==0 || + data->block[0 ] >I2C_SMBUS_BLOCK_MAX ) { + ret = -EINVAL ; +break ; + }if (data->block[0 ] >256 - command)/* Avoid overrun */ data->block[0 ] =256 - command; len = data->block[0 ];
Signature · 1 line
2.43.0
Wolfram Sang
AppliedQuoted 33 lines · Weiming Shi
The
I2C_SMBUS_I2C_BLOCK_DATAcase instub_xfer()uses data->block[0] as the transfer length. The existing check only clamps it to avoid overrunning the chip->words[256] register array, but does not validate it againstI2C_SMBUS_BLOCK_MAX(32), which is the limit of the union i2c_smbus_data.block buffer (34 bytes total). The driver is a development/test tool (CONFIG_I2C_STUB=m, not built by default) that must be loaded with a chip_addr= parameter.A local user with access to
/dev/i2c-*can issue anI2C_SMBUSioctl withI2C_SMBUS_I2C_BLOCK_DATAand data->block[0] > 32, causingstub_xfer()to read or write past the end of the union i2c_smbus_data.block buffer:BUG: KASAN: stack-out-of-bounds in stub_xfer (drivers/i2c/i2c-stub.c:223) Read of size 1 at addr ffff88800abcfd92 by task exploit/81 Call Trace: <TASK> stub_xfer (drivers/i2c/i2c-stub.c:223) __i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:593) i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:536) i2cdev_ioctl_smbus (drivers/i2c/i2c-dev.c:391) i2cdev_ioctl (drivers/i2c/i2c-dev.c:478) __x64_sys_ioctl (fs/ioctl.c:583) do_syscall_64 (arch/x86/entry/syscall_64.c:94) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) </TASK>The bug exists because i2c-stub implements .smbus_xfer directly, bypassing the
I2C_SMBUS_BLOCK_MAXvalidation ini2c_smbus_xfer_emulated(). TheI2C_SMBUS_BLOCK_DATAcase in the same function correctly validates againstI2C_SMBUS_BLOCK_MAX, but theI2C_SMBUS_I2C_BLOCK_DATAcase does not.
Quoted 8 lines
Fix by rejecting transfers with data->block[0] == 0 or data->block[0] >
I2C_SMBUS_BLOCK_MAXwith-EINVAL, consistent with both theI2C_SMBUS_BLOCK_DATAcase in the same function and theI2C_SMBUS_I2C_BLOCK_DATAvalidation ini2c_smbus_xfer_emulated().Fixes 4710317891e4 ("i2c-stub: Implement I2C block support")Reported-by Xiang Mei <[email protected]>Signed-off-by Weiming Shi <[email protected]>
Applied to for-current, thanks!
Hi Weiming,
Quoted 33 lines · Weiming Shi
The
I2C_SMBUS_I2C_BLOCK_DATAcase instub_xfer()uses data->block[0] as the transfer length. The existing check only clamps it to avoid overrunning the chip->words[256] register array, but does not validate it againstI2C_SMBUS_BLOCK_MAX(32), which is the limit of the union i2c_smbus_data.block buffer (34 bytes total). The driver is a development/test tool (CONFIG_I2C_STUB=m, not built by default) that must be loaded with a chip_addr= parameter.A local user with access to
/dev/i2c-*can issue anI2C_SMBUSioctl withI2C_SMBUS_I2C_BLOCK_DATAand data->block[0] > 32, causingstub_xfer()to read or write past the end of the union i2c_smbus_data.block buffer:BUG: KASAN: stack-out-of-bounds in stub_xfer (drivers/i2c/i2c-stub.c:223) Read of size 1 at addr ffff88800abcfd92 by task exploit/81 Call Trace: <TASK> stub_xfer (drivers/i2c/i2c-stub.c:223) __i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:593) i2c_smbus_xfer (drivers/i2c/i2c-core-smbus.c:536) i2cdev_ioctl_smbus (drivers/i2c/i2c-dev.c:391) i2cdev_ioctl (drivers/i2c/i2c-dev.c:478) __x64_sys_ioctl (fs/ioctl.c:583) do_syscall_64 (arch/x86/entry/syscall_64.c:94) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) </TASK>The bug exists because i2c-stub implements .smbus_xfer directly, bypassing the
I2C_SMBUS_BLOCK_MAXvalidation ini2c_smbus_xfer_emulated(). TheI2C_SMBUS_BLOCK_DATAcase in the same function correctly validates againstI2C_SMBUS_BLOCK_MAX, but theI2C_SMBUS_I2C_BLOCK_DATAcase does not.
Quoted 8 lines
Fix by rejecting transfers with data->block[0] == 0 or data->block[0] >
I2C_SMBUS_BLOCK_MAXwith-EINVAL, consistent with both theI2C_SMBUS_BLOCK_DATAcase in the same function and theI2C_SMBUS_I2C_BLOCK_DATAvalidation ini2c_smbus_xfer_emulated().Fixes 4710317891e4 ("i2c-stub: Implement I2C block support")Reported-by Xiang Mei <[email protected]>Signed-off-by Weiming Shi <[email protected]>
Quoted 13 lines
---
v2:
- Also reject data->block[0] == 0 (suggested by Jean Delvare).drivers/i2c/i2c-stub.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/i2c/i2c-stub.c +0
-0@@ -214,6 +214,11 @@ static s32 stub_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags,* We ignore banks here, because banked chips don't use I2C
drivers/i2c/i2c-stub.c static s32 stub_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags, · line 224
Quoted 10 lines
drivers/i2c/i2c-stub.c +5
-0* block transfers */ +if (data->block[0 ] ==0 || + data->block[0 ] >I2C_SMBUS_BLOCK_MAX ) { + ret = -EINVAL ; +break ; + }if (data->block[0 ] >256 - command)/* Avoid overrun */ data->block[0 ] =256 - command; len = data->block[0 ];
Thanks,
Signature · 2 lines
Jean Delvare SUSE L3 Support