Re: [PATCH net] eth: fbnic: fix double-free of PCS on phylink creation failure

[Bobby Eshleman]

On Fri, Apr 17, 2026 at 01:48:38AM +0200, Andrew Lunn wrote:
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > index e3ca5fcfabef..2a6a73393732 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > @@ -818,7 +818,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
> >  	netif_tx_stop_all_queues(netdev);
> >  
> >  	if (fbnic_phylink_create(netdev)) {
> > -		fbnic_netdev_free(fbd);
> > +		free_netdev(netdev);
> > +		fbd->netdev = NULL;
> 
> Why set it to NULL? Setting pointers to NULL like this often suggests
> you are not confident the code is correct and you are being
> defensive. It is better to review the code and be sure it does the
> correct thing.
> 
> >  		return NULL;
> >  	}
> >  
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > index 09c5225111be..50240e6c2ee9 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > @@ -237,6 +237,7 @@ int fbnic_phylink_create(struct net_device *netdev)
> >  		dev_err(netdev->dev.parent,
> >  			"Failed to create Phylink interface, err: %d\n", err);
> >  		xpcs_destroy_pcs(pcs);
> > +		fbn->pcs = NULL;
> 
> Why set it to NULL? If it failed, you are unwinding and about to fail
> the probe. Nothing should be using it.
> 
> I would also say fbnic_phylink_destroy() is wrong or at least whoever
> wrote it is not confident in there own code. It should only be called
> if fbnic_phylink_create() was successful, so you know fbn->pcs is
> valid, so there is no need to test it. The same for fbn->phylink.
> 
>        Andrew

Fair points. I think it looks sound without resetting to NULL, but I'll
double check and remove if confident.

I'll look at the checks in _destroy() too.

Thanks,
Bobby