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

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
eth: fbnic: move aui and fec from fbnic_net to fbnic_dev

This patch moves the aui and fec fields from struct fbnic_net to
struct fbnic_dev. It aims to prevent a time-of-check to time-of-use bug by
allowing the MDIO layer to read these fields directly from the device
structure instead of going through a potentially freed netdev.

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
> index 709041f7fc43d..49895430c97d9 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
[ ... ]
> @@ -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?
-- 
pw-bot: cr