Re: [PATCH] net: sun4i-emac: fix missing of_node_put() for phy_node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional call to `dma_release_channel()` with a potentially NULL pointer in the `emac_probe()` error path causes a kernel panic.
- [High] Premature release of the DMA channel before `unregister_netdev()` in `emac_remove()` creates a use-after-free race condition.
--

commit c4716a5c5ccd2a56d9acba6cfcd5d61c3d288e5f
Author: Li Youhong <liyouhong@kylinos.cn>

net: sun4i-emac: fix missing of_node_put() for phy_node

This commit fixes a reference leak on the PHY node in the sun4i-emac driver.
It adds the missing of_node_put() calls on the probe error path and during
device removal to properly decrement the reference count elevated by
of_parse_phandle().

> diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
> index 942454e294882..0ba67a55705d6 100644
> --- a/drivers/net/ethernet/allwinner/sun4i-emac.c
> +++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
> @@ -1067,6 +1067,7 @@ static int emac_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_release_sram:
> +	of_node_put(db->phy_node);
>  	sunxi_sram_release(&pdev->dev);
>  out_clk_disable_unprepare:
>  	clk_disable_unprepare(db->clk);

[Severity: High]
This is a pre-existing issue, but just below these lines in the error
handling path of emac_probe(), the code calls:

out_dispose_mapping:
        irq_dispose_mapping(ndev->irq);
        dma_release_channel(db->rx_chan);

Can this unconditional call to dma_release_channel() cause a kernel panic
due to a NULL pointer dereference? If emac_configure_dma() fails to acquire
a DMA channel earlier in emac_probe(), it sets db->rx_chan to NULL and allows
fallback to non-DMA mode. If a subsequent step like devm_clk_get() then
fails, execution jumps to out_dispose_mapping and dereferences db->rx_chan
without checking if it is NULL.

> @@ -1094,6 +1095,7 @@ static void emac_remove(struct platform_device *pdev)
>  	}
>  
>  	unregister_netdev(ndev);
> +	of_node_put(db->phy_node);
>  	sunxi_sram_release(&pdev->dev);
>  	clk_disable_unprepare(db->clk);
>  	irq_dispose_mapping(ndev->irq);

[Severity: High]
This is a pre-existing issue, but looking at the context right above this in
emac_remove():

    if (db->rx_chan) {
        dmaengine_terminate_all(db->rx_chan);
        dma_release_channel(db->rx_chan);
    }

    unregister_netdev(ndev);

Does releasing the DMA channel before unregistering the network device create
a use-after-free race condition?

The network interface is still logically UP when dma_release_channel() is
called, meaning its IRQ handler (emac_interrupt) is active until
unregister_netdev() completes. If an incoming packet triggers a receive
interrupt in this window, the handler calls emac_rx(), which checks
if db->rx_chan is non-NULL. Since db->rx_chan is not cleared after release,
would this submit a new DMA transaction using the freed channel structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904080758.2432748-1-dayou5941@163.com?part=1