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

[Bobby Eshleman]

On Thu, May 28, 2026 at 05:45:07PM -0700, Jakub Kicinski wrote:
> 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.

Sounds like a plan, I'm all for getting it right in one go.  Will revise
for next rev.

Best,
Bobby