linux-kernel - Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aWe7e_guDDAJdCA6@zenone.zhora.eu>
Date: Wed, 14 Jan 2026 16:52:38 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
Cc: "o.rempel@...gutronix.de" <o.rempel@...gutronix.de>, 
	"kernel@...gutronix.de" <kernel@...gutronix.de>, "shawnguo@...nel.org" <shawnguo@...nel.org>, 
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>, "festevam@...il.com" <festevam@...il.com>, 
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	GEO-CHHER-bsp-development <bsp-development.geo@...ca-geosystems.com>
Subject: Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length

Hi,

...

> > > master transmit mode before stopping. This is done by draining the
> > > pending received byte from I2DR, setting I2CR_MTX to enter transmit
> > > mode, waiting briefly for the mode change, and then proceeding with
> > > the normal STOP sequence.
> > >
> > > This change has been tested on i.MX 8M Plus platform.
> > >
> > > Signed-off-by: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
> > 
> > Is this a fix?
> 
> Yes, this is a fix. Without this patch, zero-length SMBus block reads cause a system crash 
> and permanent bus lockup on i.MX 8M Plus. The fix ensures proper STOP generation and 
> prevents buffer overruns.

Then, please add the Fixes tag.

> > > ---
> > >  drivers/i2c/busses/i2c-imx.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-imx.c
> > > b/drivers/i2c/busses/i2c-imx.c index dcce882f3eba..f40deecf0f66 100644
> > > --- a/drivers/i2c/busses/i2c-imx.c
> > > +++ b/drivers/i2c/busses/i2c-imx.c
> > > @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct
> > *i2c_imx, bool atomic)
> > >               temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > >               if (!(temp & I2CR_MSTA))
> > >                       i2c_imx->stopped = 1;
> > > +             if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
> > > +                     (void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > 
> > why do we need a cast here?
> 
> The void cast explicitly marks this as a dummy read. We're reading I2DR solely to drain the
> receive buffer (a required side effect before mode switching), not because we need the value.
> This makes the intent clear to both the compiler and code reviewers.

Please drop it.

> > > +                     temp |= I2CR_MTX;
> > > +                     imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > > +                     if (atomic)
> > > +                             udelay(25);
> > 
> > where is this 25 coming from?
> 
> It's a conservative value derived from practical testing. at standard mode (100kHz),
> this equals ~2.5 I2C clock periods; at fast mode (400kHz), ~10 periods

Please add a comment.

Andi