netdev - Re: [PATCH net] net: dsa: qca8k: forbid management frames access to internal PHYs if another device is on the MDIO bus

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: <eetsgqoq2ztgeo34kvfi732qkpegujwiy5uavpc4jognzy4mrl@owxpxsrvlwhv>
Date: Fri, 25 Apr 2025 18:29:31 +0200
From: Marek Behún <kabel@...nel.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: Marek Behún <kabel@...nel.org>, netdev@...r.kernel.org, 
	Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>, 
	"Rob Herring (Arm)" <robh@...nel.org>, Javier Carrasco <javier.carrasco.cruz@...il.com>, 
	Wojciech Drewek <wojciech.drewek@...el.com>, Matthias Schiffer <mschiffer@...verse-factory.net>, 
	Christian Marangi <ansuelsmth@...il.com>, Marek Mojík <marek.mojik@....cz>
Subject: Re: [PATCH net] net: dsa: qca8k: forbid management frames access to
 internal PHYs if another device is on the MDIO bus

On Fri, Apr 25, 2025 at 06:04:43PM +0200, Andrew Lunn wrote:
> On Fri, Apr 25, 2025 at 05:13:09PM +0200, Marek Behún wrote:
> > Completely forbid access to the internal switch PHYs via management
> > frames if there is another MDIO device on the MDIO bus besides the QCA8K
> > switch.
> > 
> > It seems that when internal PHYs are accessed via management frames,
> > internally the switch core translates these requests to MDIO accesses
> > and communicates with the internal PHYs via the MDIO bus. This
> > communication leaks outside on the MDC and MDIO pins. If there is
> > another PHY device connected on the MDIO bus besides the QCA8K switch,
> > and the kernel tries to communicate with the PHY at the same time, the
> > communication gets corrupted.
> > 
> > This makes the WAN PHY break on the Turris 1.x device.
> 
> Let me see if i understand the architecture correctly...
> 
> You have a host MDIO bus, which has both the QCA8K and a PHY on it.
> 
> Within the QCA8K there are a number of PHYs. Looking at qca8k-8xxx.c,
> there is qca8k_mdio_write() and qca8k_mdio_read() which use a register
> within the switch to access an MDIO bus for the internal PHYs. The
> assumption is that the internal MDIO is isolated from the host MDIO
> bus. That should be easy to prove, just read register 2 and 3 from all
> 32 addresses of the host MDIO bus, and make sure you don't find the
> internal PHYs.

I did not think of doing this test to determine whether the buses are
isolated when I debugged this issue and came up with commit
526c8ee04bdbd4d8.

I assumed that the buses are not isolated because I saw activity on the
bus on an oscilloscope when accessing the PHYs over Ethernet frames.

Please look at the commit message of 526c8ee04bdbd4d8
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/dsa/qca?id=526c8ee04bdbd4d8

> The issue is that when you use MDIO over Ethernet frames, both the
> internal and external bus see the bus transaction. This happens
> asynchronously to whatever the host MDIO bus driver is doing, and you
> sometimes get collisions. This in theory should affect both the PHY
> and the switch on the bus, but maybe because of the address being
> accessed, you don't notice any issue with the switch?

Yes, maybe, but also maybe because after the above mentioned commit,
the access to the MDIO bus was locked when accessing over Ethernet
frames. That was how I tried to fix this in 526c8ee04bdbd4d8.

> The assumption is that the switch hardware interpreting the MDIO over
> Ethernet is driving both the internal and external MDIO bus. Again,
> this should be easy to prove, read ID registers 2 and 3 from the
> address the external PHY is on, and see what you get.

I can try this test, but the result will change the need for this
patch.

> I guess the real question here is, has it been proven that there are
> actually two MDIO busses, not one bus with two masters? I could
> theoretically see an architecture where the switch is not managed over
> MDIO at all. It has an EEPROM to do sufficient configuration that you
> can do all the management using Ethernet frames. The MDIO over
> Ethernet then allow you to manage any external PHYs.

>From the ASCII art in commit 526c8ee04bdbd4d8 you can clearly see that
I just assumed there is just one MDIO bus, because I saw activity on
an oscilloscope. I didn't test it the way you now suggested. I also
didn't think of the consequences for the driver design and
device-trees. If we prove that there is just one MDIO bus with two
masters instead of two separate buses that leak some noise in some
situations, the situation will become more complicated...

Marek