netdev - Re: [PATCH 1/4] net: sfp: add workaround for Realtek RTL8672 and RTL9601C chips

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: <20201230174307.lvehswvj5q6c6vk3@pali>
Date:   Wed, 30 Dec 2020 18:43:07 +0100
From:   Pali Rohár <pali@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Marek Behún <kabel@...nel.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] net: sfp: add workaround for Realtek RTL8672 and
 RTL9601C chips

On Wednesday 30 December 2020 18:13:15 Andrew Lunn wrote:
> On Wed, Dec 30, 2020 at 05:05:46PM +0000, Russell King - ARM Linux admin wrote:
> > On Wed, Dec 30, 2020 at 05:56:34PM +0100, Pali Rohár wrote:
> > > This change is really required for those Realtek chips. I thought that
> > > it is obvious that from *both* addresses 0x50 and 0x51 can be read only
> > > one byte at the same time. Reading 2 bytes (for be16 value) cannot be
> > > really done by one i2 transfer, it must be done in two.
> > 
> > Then these modules are even more broken than first throught, and
> > quite simply it is pointless supporting the diagnostics on them
> > because we can never read the values in an atomic way.
> > 
> > It's also a violation of the SFF-8472 that _requires_ multi-byte reads
> > to read these 16 byte values atomically. Reading them with individual
> > byte reads results in a non-atomic read, and the 16-bit value can not
> > be trusted to be correct.
> 
> Hi Pali
> 
> I have to agree with Russell here. I would rather have no diagnostics
> than untrustable diagnostics.

Ok!

So should we completely skip hwmon_device_register_with_info() call
if (i2c_block_size < 2) ?

> The only way this is going to be accepted is if the manufacture says
> that reading the first byte of a word snapshots the second byte as
> well in an atomic way and returns that snapshot on the second
> read. But i highly doubt that happens, given how bad these SFPs are.

I do not think that manufacture says something. I think that they even
do not know that their Realtek chips are completely broken.

I can imagine that vendor just says: it is working in our branded boxes
with SFP cages and if it does not work in your kernel then problem is
with your custom kernel and we do not care about 3rd parties.