netdev - Re: [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device 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]

Date: Tue, 5 Dec 2023 14:23:16 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Serge Semin <fancer.lancer@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Jose Abreu <Jose.Abreu@...opsys.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Tomer Maimon <tmaimon77@...il.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	openbmc@...ts.ozlabs.org, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS
 MDIO-device support

On Tue, Dec 05, 2023 at 02:35:46PM +0300, Serge Semin wrote:
> Omg, thank you very much for testing the series straight away and
> sorry for the immediate trouble it caused. I'll need some more time
> for investigation. I'll get back to this topic a bit later on this
> week.

Don't worry, I got suspicious when I was CCed to review only a one-line
change in patch 11/16. It's never about that one line, is it?)

Anyway, the NULL dev->p is a symptom of device_add() not having been
called, most likely from mdio_device_register().

I'll be honest and say that I still don't quite understand what you're
trying to achieve. You're trying to bind the hardcoded mdio_devices
created by xpcs_create() to a driver? That was attempted a while ago by
Sean Anderson with the Lynx PCS. Are you aware of the fact that even in
the good case in which binding the driver actually works, the user can
then come along and unbind it from the PCS device, and phylink isn't
prepared to handle that, so it will crash the kernel upon the next
phylink_pcs call?

The pcs-rzn1-miic.c driver puts a device_link to the MAC to at least
tear down the whole thing when the PCS is unbound, which is saner than
crashing the kernel. I don't see the equivalent protection mechanism here?

Can't the xpcs continue to live without a bound driver? Having a
compatible string in the OF description is perfectly fine though,
and should absolutely not preclude that.