public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX
@ 2026-03-29 16:41 Weiming Shi
  2026-04-14 15:22 ` Jean Delvare
  0 siblings, 1 reply; 3+ messages in thread
From: Weiming Shi @ 2026-03-29 16:41 UTC (permalink / raw)
  To: Wolfram Sang, Jean Delvare; +Cc: linux-i2c, Xiang Mei, Weiming Shi, stable

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 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().

Fixes: 4710317891e4 ("i2c-stub: Implement I2C block support")
Cc: stable@vger.kernel.org
Reported-by: Xiang Mei <xmei5@asu.edu>
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];
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX
  2026-03-29 16:41 [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX Weiming Shi
@ 2026-04-14 15:22 ` Jean Delvare
  2026-04-14 17:05   ` Weiming Shi
  0 siblings, 1 reply; 3+ messages in thread
From: Jean Delvare @ 2026-04-14 15:22 UTC (permalink / raw)
  To: Weiming Shi; +Cc: Wolfram Sang, linux-i2c, Xiang Mei, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX
  2026-04-14 15:22 ` Jean Delvare
@ 2026-04-14 17:05   ` Weiming Shi
  0 siblings, 0 replies; 3+ messages in thread
From: Weiming Shi @ 2026-04-14 17:05 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Wolfram Sang, linux-i2c, Xiang Mei, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-14 17:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-29 16:41 [PATCH] i2c: stub: Reject I2C block transfers exceeding I2C_SMBUS_BLOCK_MAX Weiming Shi
2026-04-14 15:22 ` Jean Delvare
2026-04-14 17:05   ` Weiming Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox