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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240911162834.6ta45exyhbggujwl@skbuf>
Date: Wed, 11 Sep 2024 19:28:34 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "A. Sverdlin" <alexander.sverdlin@...mens.com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	stable@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: lan9303: avoid dsa_switch_shutdown()

Hi Alexander,

On Wed, Sep 11, 2024 at 04:40:03PM +0200, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> 
> dsa_switch_shutdown() doesn't bring down any ports, but only disconnects
> slaves from master. Packets still come afterwards into master port and the
> ports are being polled for link status. This leads to crashes:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> CPU: 0 PID: 442 Comm: kworker/0:3 Tainted: G O 6.1.99+ #1
> Workqueue: events_power_efficient phy_state_machine
> pc : lan9303_mdio_phy_read
> lr : lan9303_phy_read
> Call trace:
>  lan9303_mdio_phy_read
>  lan9303_phy_read
>  dsa_slave_phy_read
>  __mdiobus_read
>  mdiobus_read
>  genphy_update_link
>  genphy_read_status
>  phy_check_link_status
>  phy_state_machine
>  process_one_work
>  worker_thread
> 
> Call lan9303_remove() instead to really unregister all ports before zeroing
> drvdata and dsa_ptr.
> 
> Fixes: 0650bf52b31f ("net: dsa: be compatible with masters which unregister on shutdown")
> Cc: stable@...r.kernel.org
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> ---
>  drivers/net/dsa/lan9303-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 268949939636..ecd507355f51 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1477,7 +1477,7 @@ EXPORT_SYMBOL(lan9303_remove);
>  
>  void lan9303_shutdown(struct lan9303 *chip)
>  {
> -	dsa_switch_shutdown(chip->ds);
> +	lan9303_remove(chip);
>  }
>  EXPORT_SYMBOL(lan9303_shutdown);
>  
> -- 
> 2.46.0
> 

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?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ