Re: [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX

[Weiming Shi <bestswngs@gmail.com>]

On 26-04-14 17:22, Jean Delvare wrote:
> Hi Weiming,
> 
> On Mon, 30 Mar 2026 00:41:27 +0800, Weiming Shi wrote:
> > 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.
> 
> Thank you for the excellent analysis and detailed description. I agree
> with everything you wrote above.
> 
> > Fix by rejecting oversized transfers with -EINVAL when
> > data->block[0] exceeds I2C_SMBUS_BLOCK_MAX, 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().
> 
> Would it make sense to also reject len == 0? That's what the i2c-stub
> driver does in the I2C_SMBUS_BLOCK_DATA case, so it would seem
> consistent to do the same for the I2C_SMBUS_I2C_BLOCK_DATA case.
> 
> > 
> > Fixes: 4710317891e4 ("i2c-stub: Implement I2C block support")
> > Cc: stable@vger.kernel.org
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> 
> If there any public link to that report? If so, it should be mentioned
> here with a Closes: tag.
> 
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  drivers/i2c/i2c-stub.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-stub.c b/drivers/i2c/i2c-stub.c
> > index fbb0db41b10e1..349ef9fb2fdbc 100644
> > --- a/drivers/i2c/i2c-stub.c
> > +++ b/drivers/i2c/i2c-stub.c
> > @@ -214,6 +214,10 @@ 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] > I2C_SMBUS_BLOCK_MAX) {
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> >  		if (data->block[0] > 256 - command)	/* Avoid overrun */
> >  			data->block[0] = 256 - command;
> >  		len = data->block[0];
> 
> Reviewed-by: Jean Delvare <jdelvare@suse.de>
> 
> -- 
> Jean Delvare
> SUSE L3 Support

Hi Jean,

Thanks for the review.
I'll add a data->block[0] == 0 check to reject zero-lenght in v2.

The report no publick URL. sorry

Thanks,
Weiming Shi