linux-kernel - Re: [PATCH net-next v2 1/2] net: phy: sfp: Add support for SMBus module access

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: <caa65ad9-9489-4d22-9e87-dd30e4e16cca@lunn.ch>
Date: Tue, 25 Feb 2025 15:58:31 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: davem@...emloft.net, Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Russell King <linux@...linux.org.uk>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
	Florian Fainelli <f.fainelli@...il.com>,
	Köry Maincent <kory.maincent@...tlin.com>,
	Simon Horman <horms@...nel.org>,
	Romain Gantois <romain.gantois@...tlin.com>,
	Antoine Tenart <atenart@...nel.org>,
	Marek Behún <kabel@...nel.org>,
	Sean Anderson <sean.anderson@...ux.dev>,
	Bjørn Mork <bjorn@...k.no>
Subject: Re: [PATCH net-next v2 1/2] net: phy: sfp: Add support for SMBus
 module access

> You might be correct. As I have been running that code out-of-tree for
> a while, I was thinking that surely I'd have noticed if this was
> wrong, however there are only a few cases where we actually write to
> SFP :
> 
>  - sfp_modify_u8(...) => one-byte write
>  - in sfp_cotsworks_fixup_check(...) there are 2 writes : one 1-byte
> write and a 3-bytes write.
> 
> As I don't have any cotsworks SFP, then it looks like having the writes
> mis-ordered would have stayed un-noticed on my side as I only
> stressed the 1 byte write path...
> 
> So, good catch :) Let me triple-check and see if I can find any
> conceivable way of testing that...

Read might be more important than write. This is particularly
important for the second page containing the diagnostics, and dumped
by ethtool -m. It could be the sensor values latch when you read the
higher byte, so you can read the lower byte without worrying about it
changing. This is why we don't want HWMON, if you can only do byte
access. You might be able to test this with the temperature
sensor. The value is in 1/256 degrees. So if you can get is going from
21 255/256C to 22 0/256C and see if you ever read 21 0/256 or 22
255/256C.

	Andrew