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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 26 Mar 2022 13:39:29 +0100 From: Lukas Wunner <lukas@...ner.de> To: Andrew Lunn <andrew@...n.ch> Cc: Oleksij Rempel <o.rempel@...gutronix.de>, Oliver Neukum <oneukum@...e.com>, Oleksij Rempel <linux@...pel-privat.de>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Heiner Kallweit <hkallweit1@...il.com> Subject: Re: ordering of call to unbind() in usbnet_disconnect On Mon, Mar 21, 2022 at 02:10:27PM +0100, Andrew Lunn wrote: > There are two patterns in use at the moment: > > 1) The phy is attached in open() and detached in close(). There is no > danger of the netdev disappearing at this time. > > 2) The PHY is attached during probe, and detached during release. > > This second case is what is being used here in the USB code. This is > also a common pattern for complex devices. In probe, you get all the > components of a complex devices, stitch them together and then > register the composite device. During release, you unregister the > composite device, and then release all the components. Since this is a > natural model, i think it should work. I've gone through all drivers and noticed that some of them use a variation of pattern 2 which looks fishy: On probe, they first attach the PHY, then register the netdev. On remove, they detach the PHY, then unregister the netdev. Is it legal to detach the PHY from a registered (potentially running) netdev? It looks wrong to me. Affected drivers: drivers/net/ethernet/actions/owl-emac.c drivers/net/ethernet/altera/altera_tse_main.c drivers/net/ethernet/apm/xgene-v2/mdio.c drivers/net/ethernet/apm/xgene/xgene_enet_hw.c drivers/net/ethernet/arc/emac_main.c drivers/net/ethernet/asix/ax88796c_main.c drivers/net/ethernet/broadcom/tg3.c drivers/net/ethernet/cortina/gemini.c drivers/net/ethernet/dnet.c drivers/net/ethernet/ethoc.c drivers/net/ethernet/hisilicon/hip04_eth.c drivers/net/ethernet/lantiq_etop.c drivers/net/ethernet/marvell/pxa168_eth.c drivers/net/ethernet/toshiba/tc35815.c Some of these use devm_register_netdev() on probe and disconnect the PHY in their ->remove hook. They're missing the fact that __device_release_driver() calls the ->remove hook before devres_release_all(). Thanks, Lukas
Powered by blists - more mailing lists