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

[Madalin Bucur]

> -----Original Message-----
> From: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> Sent: Thursday, June 18, 2026 10:55 AM
> To: Madalin Bucur <madalin.bucur@xxxxxxx>
> Cc: Sean Anderson <sean.anderson@xxxxxxxxx>; Andrew Lunn
> <andrew+netdev@xxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric
> Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo
> Abeni <pabeni@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> 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 <zhaojinming@xxxxxxxxxxxxx>
> ---
>  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@xxxxxxx>

Thanks