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

Hi Théo Lebrun,

On 9/3/2026 1:57 PM, Théo Lebrun wrote:

Hello Vineeth,

On Wed Sep 2, 2026 at 12:28 PM CEST, Vineeth Karumanchi wrote:

quoted

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.

Agreed on the patch! However as we are there, I looked at all
bp->mii_bus usage and macb_phylink_connect() might crash if bp->mii_bus
is NULL.

bp->mii_bus is NULL if
 - no children mdio DT node AND
 - DT declared fixed-link, see of_phy_is_fixed_link()

In macb_phylink_connect(), phy_find_first(bp->mii_bus) will be called if
 - phylink_of_phy_connect() fails AND
 - we have no phy-handle phandle prop

Those two overlap right?

We did not observe crash in the scenario described above.
The AI-generated response below is also consistent with our observations:

A fixed-link forces phylink_of_phy_connect() to return 0 (success), so
the phy_find_first() branch is never entered.

So in macb_phylink_connect(), the fixed-link case gives dn != NULL and
ret == 0:
if (!dn || (ret && !macb_phy_handle_exists(dn))) {   /* false || (0 &&
…) → false */
        phydev = phy_find_first(bp->mii_bus);        /* not reached */

Both disjuncts are false, so phy_find_first(bp->mii_bus) is skipped.

Thanks,

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
🙏 Vineeth