On Wed, 6 Sep 2023 00:59:22 +0200, Andi Shyti wrote: > On Tue, Sep 05, 2023 at 01:35:10PM +0200, Heiner Kallweit wrote: > > On 05.09.2023 11:11, Andi Shyti wrote: > > > On Tue, Sep 05, 2023 at 10:12:43AM +0200, Jean Delvare wrote: > > >> On Sat, 02 Sep 2023 22:10:52 +0200, Heiner Kallweit wrote: > > >>> Currently we set SMBHSTCNT_LAST_BYTE only after the host has started > > >>> receiving the last byte. If we get e.g. preempted before setting > > >>> SMBHSTCNT_LAST_BYTE, the host may be finished with receiving the byte > > >>> before SMBHSTCNT_LAST_BYTE is set. > > >>> Therefore change the code to set SMBHSTCNT_LAST_BYTE before writing > > >>> SMBHSTSTS_BYTE_DONE for the byte before the last byte. Now the code > > >>> is also consistent with what we do in i801_isr_byte_done(). > > >>> > > >>> Reported-by: Jean Delvare <jdelvare@xxxxxxxx> > > >> > > >> Note for Wolfram: checkpatch says we should insert here: > > >> > > >> Closes: https://lore.kernel.org/linux-i2c/20230828152747.09444625@endymion.delvare/ > > > > > > does this also need a Fixes: tag? I tried to check it, but there > > > was an intricate jungle of commits in these lines. > > > > > Quoting Jean from previous communication about this patch: > > As far as I see, the race condition already existed when the kernel > > switched to git, so there's no point in having a Fixes statement. > > true... I forgot about this comment. > > Anyway I think that this should, then, go to all the stable > kernels and I believe that without the Fixes tag this will never > be picked up. > > Maybe Greg can advise here. > > Would you mind resending this patch Cc'eing the stable kernel and > adding a note after the '---'? Again, no. This is a theoretical bug, that was discovered by code inspection. It's been present in the driver for over 20 years and we have no evidence that anyone ever hit it. Furthermore, it only happens if the driver is in polling mode, which is uncommon, and only for block transactions, and only when the block buffer isn't used. And then the only negative effect of the bug is to shift the internal register pointer by 1. So it would only ever be a problem if someone mixes block reads and immediate byte reads on the same device. That's a very uncommon usage model. That's definitely not "a real bug that bothers people". Not every fix needs to go to stable. In this case, our engineering time is better used elsewhere. -- Jean Delvare SUSE L3 Support
