netdev - Re: [PATCH net] net: dsa: lan9303: avoid dsa_switch

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: <06f6c7d6f1e812c862af892f89d56d74b69995f9.camel@siemens.com>
Date: Wed, 11 Sep 2024 17:04:28 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "olteanv@...il.com" <olteanv@...il.com>
CC: "andrew@...n.ch" <andrew@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "f.fainelli@...il.com" <f.fainelli@...il.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: dsa: lan9303: avoid dsa_switch_shutdown()

Hello Vladimir,

On Wed, 2024-09-11 at 18:55 +0200, Alexander Sverdlin wrote:
> > You've said here that a similar change still does not protect against
> > packets received after shutdown:
> > https://lore.kernel.org/netdev/c5e0e67400816d68e6bf90b4a999bfa28c59043b.camel@siemens.com/
> > 
> > The difference between that and this is the extra lan9303_disable_processing_port()
> > calls here. But while that does disable RX on switch ports, it still doesn't wait
> > for pending RX frames to be processed. So the race is still open. No?

besides from the below, I've expected this question... In the meanwhile I've tested
mv88e6xxx driver, but it (accidentally) has no MDIO race vs shutdown.
After some shallow review of the drivers I didn't find dev_get_drvdata <= mdio_read
pattern therefore I've posted this tested patch.

If you'd prefer to solve this centrally for all drivers, I can test your patch from
the MDIO-drvdata PoV.

> This patch addresses the race of zeroing drvdata in
> 
> static void lan9303_mdio_shutdown(struct mdio_device *mdiodev)
> {
>         struct lan9303_mdio *sw_dev = dev_get_drvdata(&mdiodev->dev);
>         
>         if (!sw_dev)
>                 return;
>  
>         lan9303_shutdown(&sw_dev->chip);
>         
>         dev_set_drvdata(&mdiodev->dev, NULL);
> }
> 
> versus 
> 
> static int lan9303_mdio_phy_read(struct lan9303 *chip, int phy,  int reg)
> {
>         struct lan9303_mdio *sw_dev = dev_get_drvdata(chip->dev);
> 
> what you refer to is another race, zeroing of dsa_ptr in struct net_device versus
> the whole network stack, which I addressed in
> https://lore.kernel.org/netdev/20240910130321.337154-2-alexander.sverdlin@siemens.com/

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com