Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Vineeth

On 2.9.2026 12:28, Vineeth Karumanchi wrote:
> When the device tree describes a fixed-link and has no "mdio" child
> node, macb_mii_init() returns early without allocating the MDIO bus,
> leaving bp->mii_bus as NULL.
> 
> Two cleanup paths then dereference this NULL bus:
> 
> 1. On driver unbind, macb_remove() unconditionally calls
>    mdiobus_unregister(bp->mii_bus), which oopses:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 
> 00000000000004a8
>   pc : mdiobus_unregister+0x14/0xa4
>   lr : macb_remove+0x38/0xa4
>   Call trace:
>    mdiobus_unregister+0x14/0xa4 (P)
>    macb_remove+0x38/0xa4
>    platform_remove+0x20/0x30
>    device_release_driver_internal+0x1c8/0x224
>    unbind_store+0xb4/0xbc
> 
> 2. On the probe error path in macb_probe(), reached when
>    macb_mii_init() has succeeded but a subsequent step fails, the
>    err_out_unregister_mdio label runs the same unconditional cleanup.
> 
> mdiobus_unregister() and mdiobus_free() do not guard against a NULL
> bus, so guard the calls in both macb_remove() and the probe error
> path.
> 
> Fixes: d0c3601f2c4e ("net: macb: Avoid 20s boot delay by skipping MDIO 
> bus registration for fixed-link PHY")
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 76ee4f506033..88eebe187eac 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -5971,8 +5971,10 @@ static int macb_probe(struct platform_device 
> *pdev)
>  	macb_free_tieoff(bp);
> 
>  err_out_unregister_mdio:
> -	mdiobus_unregister(bp->mii_bus);
> -	mdiobus_free(bp->mii_bus);
> +	if (bp->mii_bus) {
> +		mdiobus_unregister(bp->mii_bus);
> +		mdiobus_free(bp->mii_bus);
> +	}
> 
>  err_out_phy_exit:
>  	phy_exit(bp->phy);
> @@ -6001,8 +6003,10 @@ static void macb_remove(struct platform_device 
> *pdev)
>  		unregister_netdev(netdev);
>  		macb_free_tieoff(bp);
>  		phy_exit(bp->phy);
> -		mdiobus_unregister(bp->mii_bus);
> -		mdiobus_free(bp->mii_bus);
> +		if (bp->mii_bus) {
> +			mdiobus_unregister(bp->mii_bus);
> +			mdiobus_free(bp->mii_bus);
> +		}

nit: replica of the code from above, but also not really worth a helper?

> 
>  		device_set_wakeup_enable(&bp->pdev->dev, 0);
>  		cancel_delayed_work_sync(&bp->tx_lpi_work);

Anyway:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai