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

[Bobby Eshleman]

On Thu, May 07, 2026 at 07:29:54AM -0700, Jakub Kicinski wrote:
> On Thu, 7 May 2026 07:24:53 -0700 Jakub Kicinski wrote:
> > On Thu, 7 May 2026 12:34:24 +0200 Paolo Abeni wrote:
> > > > Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
> > > > callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
> > > > These callers remain active even after a failed probe, so fdb->netdev
> > > > still needs to be cleared.
> > > > 
> > > > Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@xxxxxxxx>    
> > > 
> > > Note that sashiko-gemini spotted a pre-existing issue:
> > > 
> > > https://sashiko.dev/#/patchset/20260504-fbnic-pcs-fix-v2-1-de45192821d9%40meta.com
> > > 
> > > does not block this patch but could deserve a follow-up.  
> > 
> > fbd is a devlink priv, not netdev priv, touching it after free_netdev()
> > is perfectly fine. I wish Gemini tried a *little* harder instead of
> > guessing :| Sorry for not commenting earlier.
> 
> Ugh, not enough coffee. It's complaining about MDIO reads, I think
> that's valid.

It is, but I think the race pre-exists.

static int
fbnic_mdio_read_pmd(struct fbnic_dev *fbd, int addr, int regnum)
[...]
	if (fbd->netdev) {
		fbn = netdev_priv(fbd->netdev);
		if (fbn->aui < FBNIC_AUI_UNKNOWN)
			aui = fbn->aui;
	}


Definitely possible that ->netdev gets freed concurrently with
fbd->netdev evaluating to true... but fbnic_netdev_free() faces the same
race.

I'm open to fixing this all at once, if preferred. Probably need to look
at some of the other fbnic_net ptr guards too.

Best,
Bobby