linux-kernel - Re: [PATCH v5 06/11] i2c: rtl9300: remove SMBus Quick operation support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d01e5498-b2b0-473b-b8e7-339825c45043@gmail.com>
Date: Sun, 10 Aug 2025 11:31:53 +0200
From: Jonas Jelonek <jelonek.jonas@...il.com>
To: Sven Eckelmann <sven@...fation.org>,
 Chris Packham <chris.packham@...iedtelesis.co.nz>,
 Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>
Cc: linux-i2c@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Markus Stockhausen <markus.stockhausen@....de>,
 Harshal Gohel <hg@...onwunderlich.de>
Subject: Re: [PATCH v5 06/11] i2c: rtl9300: remove SMBus Quick operation
 support



On 10.08.2025 09:13, Sven Eckelmann wrote:
> On Sunday, 10 August 2025 00:07:07 CEST Jonas Jelonek wrote:
> [...]
>> The current implementation of SMBus Quick operation passes a length of
>> 0 (which is actually invalid). Before the fix of a bug in a previous
>> commit, this led to a read operation of 16 bytes from any register (the
>> one of a former transaction or any other value.
>>
>> Although there are currently no reports of actual issues this caused.
>> However, as an example, i2cdetect by default uses Quick Write operation
>> to probe the bus and this may already write anything to some register
>> of a device, causing unintended behaviour. This could be the cause of a
>> recent brick of one of my DAC cables where there was a checksum mismatch
>> of the EEPROM after having run 'i2cdetect -l' before.
> [...]
>
> Nice find. I've actually observed odd behavior after/during probing and 
> attributed it only to the other problems (especially the low timeout + missing 
> check) we found and never did a deep dive to figure out what happened on the 
> bus during the probe. Possible that this could be related.

Haven't actually described my issue in detail in the commit message (may add
this in the next version) but it perfectly makes sense. Quick operation in the
driver passed a length of 0 to config_xfer. Internally, this leads to a value of
0xf in the DATA_WIDTH register meaning 16 bytes to be read/written because
of:

(len - 1) & 0xf

The register value obviously was assumed to be 0 by the hardware and data
was completely zeroed too. Then it did a 16-byte write of that. Unfortunately,
the EEPROM of my DAC isn't write-protected so it was written to the EEPROM
causing checksum error on next boot (was able to fix that though).

> Reviewed-by: Sven Eckelmann <sven@...fation.org>

Thanks!

> Kind regards,
> 	Sven

Best,
Jonas