* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-02 10:28 [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link Vineeth Karumanchi
@ 2026-09-03 2:17 ` Xuanqiang Luo
2026-09-03 7:23 ` Nicolai Buchwitz
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Xuanqiang Luo @ 2026-09-03 2:17 UTC (permalink / raw)
To: Vineeth Karumanchi
Cc: git, netdev, linux-kernel, theo.lebrun, conor.dooley,
andrew+netdev, davem, edumazet, kuba, pabeni
在 2026/9/2 18:28, Vineeth Karumanchi 写道:
> 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>
Reviewed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Thanks,
Xuanqiang
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-02 10:28 [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link Vineeth Karumanchi
2026-09-03 2:17 ` Xuanqiang Luo
@ 2026-09-03 7:23 ` Nicolai Buchwitz
2026-09-03 8:28 ` Théo Lebrun
2026-09-03 8:27 ` Théo Lebrun
2026-09-06 0:20 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 7+ messages in thread
From: Nicolai Buchwitz @ 2026-09-03 7:23 UTC (permalink / raw)
To: Vineeth Karumanchi
Cc: theo.lebrun, conor.dooley, andrew+netdev, davem, edumazet, kuba,
pabeni, git, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-03 7:23 ` Nicolai Buchwitz
@ 2026-09-03 8:28 ` Théo Lebrun
0 siblings, 0 replies; 7+ messages in thread
From: Théo Lebrun @ 2026-09-03 8:28 UTC (permalink / raw)
To: Nicolai Buchwitz, Vineeth Karumanchi
Cc: conor.dooley, andrew+netdev, davem, edumazet, kuba, pabeni, git,
netdev, linux-kernel
Hi Nicolai,
On Thu Sep 3, 2026 at 9:23 AM CEST, Nicolai Buchwitz wrote:
> On 2.9.2026 12:28, Vineeth Karumanchi wrote:
>> @@ -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?
Agreed that the helper is overblown!
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-02 10:28 [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link Vineeth Karumanchi
2026-09-03 2:17 ` Xuanqiang Luo
2026-09-03 7:23 ` Nicolai Buchwitz
@ 2026-09-03 8:27 ` Théo Lebrun
2026-09-03 9:37 ` Karumanchi, Vineeth
2026-09-06 0:20 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 7+ messages in thread
From: Théo Lebrun @ 2026-09-03 8:27 UTC (permalink / raw)
To: Vineeth Karumanchi, conor.dooley, andrew+netdev, davem, edumazet,
kuba, pabeni
Cc: git, netdev, linux-kernel
Hello Vineeth,
On Wed Sep 2, 2026 at 12:28 PM CEST, 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.
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?
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-03 8:27 ` Théo Lebrun
@ 2026-09-03 9:37 ` Karumanchi, Vineeth
0 siblings, 0 replies; 7+ messages in thread
From: Karumanchi, Vineeth @ 2026-09-03 9:37 UTC (permalink / raw)
To: Théo Lebrun, Vineeth Karumanchi, conor.dooley,
andrew+netdev, davem, edumazet, kuba, pabeni
Cc: git, netdev, linux-kernel
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:
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link
2026-09-02 10:28 [PATCH net] net: macb: fix NULL pointer dereference on unbind with fixed-link Vineeth Karumanchi
` (2 preceding siblings ...)
2026-09-03 8:27 ` Théo Lebrun
@ 2026-09-06 0:20 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-06 0:20 UTC (permalink / raw)
To: Vineeth Karumanchi
Cc: theo.lebrun, conor.dooley, andrew+netdev, davem, edumazet, kuba,
pabeni, git, netdev, linux-kernel
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 2 Sep 2026 15:58:36 +0530 you 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:
>
> [...]
Here is the summary with links:
- [net] net: macb: fix NULL pointer dereference on unbind with fixed-link
https://git.kernel.org/netdev/net/c/38b6be101006
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread