[PATCH net] net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

[PATCH net] net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

mdio_mux_init() registers the child MDIO buses one after the other.
Scanning the internal bus makes mdio_mux_syscon_switch_fn() select the
internal PHY and power it, which sets gmac->internal_phy_powered. If
mdio_mux_init() then fails without switching away from the internal
bus, for example because registering the external bus returns
-EPROBE_DEFER before any access to it, or because an allocation for it
fails, sun8i_dwmac_probe() jumps to dwmac_mux with the PHY still
powered.

That error path releases gmac->rst_ephy and gmac->ephy_clk and only
then calls stmmac_pltfr_remove(), which ends in sun8i_dwmac_exit().
Since the PHY is still marked as powered, sun8i_dwmac_exit() calls
sun8i_dwmac_unpower_internal_phy(), which passes the clock and the
reset control that were just freed to clk_disable_unprepare() and
reset_control_assert().

Unpower the internal PHY before releasing its clock and reset control,
as sun8i_dwmac_remove() does. The later call from sun8i_dwmac_exit()
then does nothing because internal_phy_powered is already cleared.

Fixes: 529254216773 ("net: stmmac: dwmac-sun8i: Balance internal PHY resource references")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
The problem was pointed out by the Sashiko AI review of an unrelated
dwmac-sun8i series of mine. It holds against the code: clk_put() and
reset_control_put() free the per-consumer struct clk and the exclusive
reset control, and nothing between the dwmac_mux label and
sun8i_dwmac_exit() clears internal_phy_powered.

This is build-tested only (arm64 and arm, W=1). I have no board with the
internal PHY (H3, H5 or V3s), so the error path was not exercised.

This patch was developed with Claude (Anthropic) as a coding assistant:
it checked the report against the code and wrote the fix and the commit
message. I reviewed it.
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 47954537b29e..4436532af2dc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1245,6 +1245,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	return 0;
 
 dwmac_mux:
+	sun8i_dwmac_unpower_internal_phy(gmac);
 	reset_control_put(gmac->rst_ephy);
 	clk_put(gmac->ephy_clk);
 dwmac_remove:

---
base-commit: 145480b28ca82e9fae789427cc4569e596515a32
change-id: 20261112-sun8i-emac-mux-uaf-1ba885219fee

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

[PATCH net] net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

[Arun Iyengar]

mdio_mux_init() registers the child MDIO buses one after the other.
Scanning the internal bus makes mdio_mux_syscon_switch_fn() select the
internal PHY and power it, which sets gmac->internal_phy_powered. If
mdio_mux_init() then fails without switching away from the internal
bus, for example because registering the external bus returns
-EPROBE_DEFER before any access to it, or because an allocation for it
fails, sun8i_dwmac_probe() jumps to dwmac_mux with the PHY still
powered.

That error path releases gmac->rst_ephy and gmac->ephy_clk and only
then calls stmmac_pltfr_remove(), which ends in sun8i_dwmac_exit().
Since the PHY is still marked as powered, sun8i_dwmac_exit() calls
sun8i_dwmac_unpower_internal_phy(), which passes the clock and the
reset control that were just freed to clk_disable_unprepare() and
reset_control_assert().

Unpower the internal PHY before releasing its clock and reset control,
as sun8i_dwmac_remove() does. The later call from sun8i_dwmac_exit()
then does nothing because internal_phy_powered is already cleared.

Fixes: 529254216773 ("net: stmmac: dwmac-sun8i: Balance internal PHY resource references")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
The problem was pointed out by the Sashiko AI review of an unrelated
dwmac-sun8i series of mine. It holds against the code: clk_put() and
reset_control_put() free the per-consumer struct clk and the exclusive
reset control, and nothing between the dwmac_mux label and
sun8i_dwmac_exit() clears internal_phy_powered.

This is build-tested only (arm64 and arm, W=1). I have no board with the
internal PHY (H3, H5 or V3s), so the error path was not exercised.

This patch was developed with Claude (Anthropic) as a coding assistant:
it checked the report against the code and wrote the fix and the commit
message. I reviewed it.
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 47954537b29e..4436532af2dc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1245,6 +1245,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	return 0;
 
 dwmac_mux:
+	sun8i_dwmac_unpower_internal_phy(gmac);
 	reset_control_put(gmac->rst_ephy);
 	clk_put(gmac->ephy_clk);
 dwmac_remove:

---
base-commit: 145480b28ca82e9fae789427cc4569e596515a32
change-id: 20261112-sun8i-emac-mux-uaf-1ba885219fee

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

Re: [PATCH net] net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

[Andrew Lunn]

On Thu, Nov 12, 2026 at 05:28:05AM +0000, Arun Iyengar via B4 Relay wrote:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> mdio_mux_init() registers the child MDIO buses one after the other.
> Scanning the internal bus makes mdio_mux_syscon_switch_fn() select the
> internal PHY and power it, which sets gmac->internal_phy_powered. If
> mdio_mux_init() then fails without switching away from the internal
> bus, for example because registering the external bus returns
> -EPROBE_DEFER before any access to it, or because an allocation for it
> fails, sun8i_dwmac_probe() jumps to dwmac_mux with the PHY still
> powered.
> 
> That error path releases gmac->rst_ephy and gmac->ephy_clk and only
> then calls stmmac_pltfr_remove(), which ends in sun8i_dwmac_exit().
> Since the PHY is still marked as powered, sun8i_dwmac_exit() calls
> sun8i_dwmac_unpower_internal_phy(), which passes the clock and the
> reset control that were just freed to clk_disable_unprepare() and
> reset_control_assert().
> 
> Unpower the internal PHY before releasing its clock and reset control,
> as sun8i_dwmac_remove() does. The later call from sun8i_dwmac_exit()
> then does nothing because internal_phy_powered is already cleared.
> 
> Fixes: 529254216773 ("net: stmmac: dwmac-sun8i: Balance internal PHY resource references")
> Cc: stable@vger.kernel.org

Does this bother anybody? Seems more like this should be for net-next.


    Andrew

---
pw-bot: cr

Re: [PATCH net] net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] net: stmmac: dwmac-sun8i: unhandled -EPROBE_DEFER in
  get_ephy_nodes() [1]
--

commit 0e3757128b0330be69abe1ee1230d93499f349af
Author: Arun Iyengar <aiyengar1990@gmail.com>

net: stmmac: dwmac-sun8i: Fix use-after-free in the MDIO mux error path

This commit unpowers the internal PHY on the sun8i_dwmac_probe() MDIO mux
error path before releasing gmac->rst_ephy and gmac->ephy_clk so that the
subsequent sun8i_dwmac_exit() call does not access freed resources.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 47954537b29e4..4436532af2dce 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -1245,6 +1245,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>  	return 0;
>  
>  dwmac_mux:
> +	sun8i_dwmac_unpower_internal_phy(gmac);
>  	reset_control_put(gmac->rst_ephy);
>  	clk_put(gmac->ephy_clk);
>  dwmac_remove:

[Severity: High]
This problem wasn't introduced by this patch, but does get_ephy_nodes()
swallow -EPROBE_DEFER when acquiring ephy_clk [1]?

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:get_ephy_nodes() {
	...
	for_each_child_of_node_scoped(mdio_internal, iphynode) {
		struct clk *ephy_clk;

		ephy_clk = of_clk_get(iphynode, 0);
		if (IS_ERR(ephy_clk))
			continue;
		gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);
		if (IS_ERR(gmac->rst_ephy)) {
			ret = PTR_ERR(gmac->rst_ephy);
			clk_put(ephy_clk);
			if (ret == -EPROBE_DEFER) {
				of_node_put(mdio_internal);
				return ret;
			}
			continue;
		}
	...
	of_node_put(mdio_internal);
	return -ENODEV;
}

Unlike the of_reset_control_get_exclusive() check right below it, an
IS_ERR(ephy_clk) result unconditionally continues the loop without checking
for -EPROBE_DEFER.  If the internal PHY clock provider is not registered
yet during boot, get_ephy_nodes() exhausts the child nodes and returns
-ENODEV, causing sun8i_dwmac_probe() to fail permanently instead of
deferring probe.

[1] https://sashiko.dev/#/bug/linux-98384c14-541e-410e-a074-fff5e98b3820

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-sun8i-emac-mux-uaf-v1-1-308ae7e6fcf2@gmail.com?part=1