Re: [PATCH net v2] eth: fbnic: move aui and fec from fbnic_net to fbnic_dev
From: Jakub Kicinski
Date: Thu May 28 2026 - 20:45:29 EST
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.