Re: [PATCH net v2] eth: fbnic: move aui and fec from fbnic_net to fbnic_dev

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 28 May 2026 17:37:14 -0700 Jakub Kicinski wrote:
> > @@ -15,18 +14,14 @@ static int
> >  fbnic_mdio_read_pmd(struct fbnic_dev *fbd, int addr, int regnum)
> >  {
> >  	u8 aui = FBNIC_AUI_UNKNOWN;
> > -	struct fbnic_net *fbn;
> >  	int ret = 0;
> >  
> >  	/* We don't need a second PMD, just one can handle both lanes */
> >  	if (addr)
> >  		return 0;
> >  
> > -	if (fbd->netdev) {
> > -		fbn = netdev_priv(fbd->netdev);
> > -		if (fbn->aui < FBNIC_AUI_UNKNOWN)
> > -			aui = fbn->aui;
> > -	}
> > +	if (fbd->aui < FBNIC_AUI_UNKNOWN)
> > +		aui = fbd->aui;  
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does moving aui and fec to
> struct fbnic_dev actually prevent a use-after-free here?
> 
> If fbnic_mdiobus_create() registers the MDIO bus using
> devm_mdiobus_register(), devres-managed resources are cleaned up after the
> driver's remove callback returns.
> 
> If fbnic_remove() frees the fbd structure, the bus remains fully active
> between the time the remove function finishes and the devres cleanup
> unregisters the MDIO bus.
> 
> Can a concurrent sysfs access invoke MDIO read callbacks which pass fbd
> to this function, leading to a dereference of the freed fbd pointer when
> reading fbd->aui?

Sorry for broadening the scope here a little bit but I think we should
fix this (as well). In the same series, but probably separate patch.
Just don't use devm_, it creates about as many bugs as it prevents.