netdev - Re: [PATCH net 2/2] net: ethernet: ftgmac100: fix NULL phy usage on device remove

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: <8780e73e-78cd-4841-8c04-b453fe664bab@lunn.ch>
Date: Mon, 28 Oct 2024 21:23:13 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Jeremy Kerr <jk@...econstruct.com.au>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Joel Stanley <joel@....id.au>,
	Jacky Chou <jacky_chou@...eedtech.com>,
	Jacob Keller <jacob.e.keller@...el.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net 2/2] net: ethernet: ftgmac100: fix NULL phy usage on
 device remove

On Mon, Oct 28, 2024 at 12:54:11PM +0800, Jeremy Kerr wrote:
> Commit e24a6c874601 ("net: ftgmac100: Get link speed and duplex for
> NC-SI") introduced a fixed phydev attached to the ftgmac netdev for ncsi
> configurations, cleaned up on remove as:
> 
>     phy_disconnect(netdev->phydev);
> 
>     /* ... */
> 
>     if (priv->use_ncsi)
>         fixed_phy_unregister(netdev->phydev);
> 
> However, phy_disconnect() will clear the netdev's ->phydev pointer, so
> the fixed_phy_unregister() will always be invoked with a null pointer.
> 
> Use a temporary for the phydev, rather than expecting the netdev->phydev
> point to be valid over the phy_disconnect().
> 
> Fixes: e24a6c874601 ("net: ftgmac100: Get link speed and duplex for NC-SI")
> Signed-off-by: Jeremy Kerr <jk@...econstruct.com.au>
> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 9caee68468ff5f71d7ea63a0c8c9ec2be4a718bc..c6ed7ed0e2389a45a671b85ae60936df99458cd1 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1730,16 +1730,17 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)
>  static void ftgmac100_phy_disconnect(struct net_device *netdev)

This all seems rather hacky. What is the mirror function to
ftgmac100_phy_disconnect(). I don't see a
ftgmac100_phy_connect(). Generally, if the teardown function does the
same as the setup function, but reverse order, it just works....

     Andrew