Thread (14 messages) 14 messages, 4 authors, 11d ago

RE: [PATCH] net: fman: fix clock and device node leak in probe error paths

From: Madalin Bucur <madalin.bucur@nxp.com>
Date: 2026-06-18 08:08:23
Also in: lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: ZhaoJinming <redacted>
Sent: Thursday, June 18, 2026 10:55 AM
To: Madalin Bucur <madalin.bucur@nxp.com>
Cc: Sean Anderson <sean.anderson@linux.dev>; Andrew Lunn
[off-list ref]; David S. Miller [off-list ref]; Eric
Dumazet [off-list ref]; Jakub Kicinski [off-list ref]; Paolo
Abeni [off-list ref]; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org; ZhaoJinming [off-list ref]
Subject: [PATCH] net: fman: fix clock and device node leak in probe error paths

In read_dts_node(), fm_node is acquired via of_node_get() and clk is
acquired via of_clk_get(). After a successful of_clk_get() call, the
error paths for clk_get_rate failure and of_property_read_u32_array
failure correctly goto fman_node_put (releasing fm_node) but miss
clk_put(clk).

Worse, all error paths from the MURAM node lookup onward goto
fman_free directly, skipping both fman_node_put and clk_put, leaking
both the fm_node and clk references.

of_clk_get() eventually calls __of_clk_get() -> clk_hw_create_clk() ->
alloc_clk() -> kzalloc_obj() to allocate the clk struct, so clk_put()
must be called to release this memory. Without it, the allocated clk
struct is leaked on every probe failure after of_clk_get() succeeds.

Introduce a clk_put label between the success return and fman_node_put
in the unwind chain, and redirect all error paths after of_clk_get()
to this new label. Since no goto target remains for fman_free, fold
it into fman_node_put and remove the now-unused label.

Signed-off-by: ZhaoJinming <redacted>
---
 drivers/net/ethernet/freescale/fman/fman.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c
b/drivers/net/ethernet/freescale/fman/fman.c
index 013273a2de32..734cbe8efd7e 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2736,7 +2736,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
                err = -EINVAL;
                dev_err(&of_dev->dev, "%s: Failed to determine FM%d clock rate\n",
                        __func__, fman->dts_params.id);
-               goto fman_node_put;
+               goto clk_put;
        }
        /* Rounding to MHz */
        fman->dts_params.clk_freq = DIV_ROUND_UP(clk_rate, 1000000);
@@ -2746,7 +2746,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
        if (err) {
                dev_err(&of_dev->dev, "%s: failed to read fsl,qman-channel-range
for %pOF\n",
                        __func__, fm_node);
-               goto fman_node_put;
+               goto clk_put;
        }
        fman->dts_params.qman_channel_base = range[0];
        fman->dts_params.num_of_qman_channels = range[1];
@@ -2757,7 +2757,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
                err = -EINVAL;
                dev_err(&of_dev->dev, "%s: could not find MURAM node\n",
                        __func__);
-               goto fman_free;
+               goto clk_put;
        }

        err = of_address_to_resource(muram_node, 0,
@@ -2766,7 +2766,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
                of_node_put(muram_node);
                dev_err(&of_dev->dev, "%s: of_address_to_resource() = %d\n",
                        __func__, err);
-               goto fman_free;
+               goto clk_put;
        }

        of_node_put(muram_node);
@@ -2776,7 +2776,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
        if (err < 0) {
                dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
                        __func__, irq, err);
-               goto fman_free;
+               goto clk_put;
        }

        if (fman->dts_params.err_irq != 0) {
@@ -2786,7 +2786,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
                if (err < 0) {
                        dev_err(&of_dev->dev, "%s: irq %d allocation failed (error
= %d)\n",
                                __func__, fman->dts_params.err_irq, err);
-                       goto fman_free;
+                       goto clk_put;
                }
        }
@@ -2794,7 +2794,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
        if (IS_ERR(base_addr)) {
                err = PTR_ERR(base_addr);
                dev_err(&of_dev->dev, "%s: devm_ioremap() failed\n", __func__);
-               goto fman_free;
+               goto clk_put;
        }

        fman->dts_params.base_addr = base_addr;
@@ -2806,7 +2806,7 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)
        if (err) {
                dev_err(&of_dev->dev, "%s: of_platform_populate() failed\n",
                        __func__);
-               goto fman_free;
+               goto clk_put;
        }

 #ifdef CONFIG_DPAA_ERRATUM_A050385
@@ -2816,9 +2816,10 @@ static struct fman *read_dts_node(struct
platform_device *of_dev)

        return fman;

+clk_put:
+       clk_put(clk);
 fman_node_put:
        of_node_put(fm_node);
-fman_free:
        kfree(fman);
        return ERR_PTR(err);
 }
--
2.20.1
Acked-by: Madalin Bucur <madalin.bucur@nxp.com>

Thanks
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help