[v2] clk: imx: imx6q: Fix device node reference leaks

[PATCH v2 1/2] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()

Posted by Felix Gu 1 week, 2 days ago

The function pll6_bypassed() calls of_parse_phandle_with_args()
but never calls of_node_put() to release the reference, causing
a memory leak.

Fix this by adding proper cleanup calls on all exit paths.

Fixes: 3cc48976e9763 ("clk: imx6q: handle ENET PLL bypass")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/clk/imx/clk-imx6q.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index bf4c1d9c9928..1d8e8f0891a3 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -238,8 +238,11 @@ static bool pll6_bypassed(struct device_node *node)
 			return false;
 
 		if (clkspec.np == node &&
-		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS)
+		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS) {
+			of_node_put(clkspec.np);
 			break;
+		}
+		of_node_put(clkspec.np);
 	}
 
 	/* PLL6 bypass is not part of the assigned clock list */
@@ -249,6 +252,7 @@ static bool pll6_bypassed(struct device_node *node)
 	ret = of_parse_phandle_with_args(node, "assigned-clock-parents",
 					 "#clock-cells", index, &clkspec);
 
+	of_node_put(clkspec.np);
 	if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
 		return true;
 

-- 
2.43.0

Re: [PATCH v2 1/2] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()

Posted by Frank Li 1 week, 1 day ago

On Sat, Jan 31, 2026 at 04:11:34PM +0800, Felix Gu wrote:
> The function pll6_bypassed() calls of_parse_phandle_with_args()
> but never calls of_node_put() to release the reference, causing
> a memory leak.
>
> Fix this by adding proper cleanup calls on all exit paths.
>
> Fixes: 3cc48976e9763 ("clk: imx6q: handle ENET PLL bypass")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/clk/imx/clk-imx6q.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index bf4c1d9c9928..1d8e8f0891a3 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -238,8 +238,11 @@ static bool pll6_bypassed(struct device_node *node)
>  			return false;
>
>  		if (clkspec.np == node &&
> -		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS)
> +		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS) {
> +			of_node_put(clkspec.np);
>  			break;
> +		}
> +		of_node_put(clkspec.np);
>  	}
>
>  	/* PLL6 bypass is not part of the assigned clock list */
> @@ -249,6 +252,7 @@ static bool pll6_bypassed(struct device_node *node)
>  	ret = of_parse_phandle_with_args(node, "assigned-clock-parents",
>  					 "#clock-cells", index, &clkspec);
>

If ret is err, clkspec will not touched. So clkspec.np keep old value,
of_node_put(clkspec.np) will put twice for previous np.

So need add if (ret) check here. Or use difference variable clkspec with
init 0.

Frank

> +	of_node_put(clkspec.np);
>  	if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
>  		return true;
>
>
> --
> 2.43.0
>

Re: [PATCH v2 1/2] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()

Posted by Peng Fan 1 week ago

On Sat, Jan 31, 2026 at 11:16:57AM -0500, Frank Li wrote:
>On Sat, Jan 31, 2026 at 04:11:34PM +0800, Felix Gu wrote:
>> The function pll6_bypassed() calls of_parse_phandle_with_args()
>> but never calls of_node_put() to release the reference, causing
>> a memory leak.
>>
>> Fix this by adding proper cleanup calls on all exit paths.
>>
>> Fixes: 3cc48976e9763 ("clk: imx6q: handle ENET PLL bypass")
>> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
>> ---
>>  drivers/clk/imx/clk-imx6q.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
>> index bf4c1d9c9928..1d8e8f0891a3 100644
>> --- a/drivers/clk/imx/clk-imx6q.c
>> +++ b/drivers/clk/imx/clk-imx6q.c
>> @@ -238,8 +238,11 @@ static bool pll6_bypassed(struct device_node *node)
>>  			return false;
>>

One more comment:
Put one line here, "of_node_put", no need duplicate it in below.

>>  		if (clkspec.np == node &&
>> -		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS)
>> +		    clkspec.args[0] == IMX6QDL_PLL6_BYPASS) {
>> +			of_node_put(clkspec.np);

Drop this change

>>  			break;
>> +		}
>> +		of_node_put(clkspec.np);

Ditto.

Regards,
Peng

>>  	}
>>
>>  	/* PLL6 bypass is not part of the assigned clock list */
>> @@ -249,6 +252,7 @@ static bool pll6_bypassed(struct device_node *node)
>>  	ret = of_parse_phandle_with_args(node, "assigned-clock-parents",
>>  					 "#clock-cells", index, &clkspec);
>>
>
>If ret is err, clkspec will not touched. So clkspec.np keep old value,
>of_node_put(clkspec.np) will put twice for previous np.
>
>So need add if (ret) check here. Or use difference variable clkspec with
>init 0.
>
>Frank
>
>> +	of_node_put(clkspec.np);
>>  	if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
>>  		return true;
>>
>>
>> --
>> 2.43.0
>>

Re: [PATCH v2 1/2] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()

Posted by Felix Gu 6 days, 18 hours ago

On Mon, Feb 2, 2026 at 10:03 AM Peng Fan <peng.fan@oss.nxp.com> wrote:
>
> On Sat, Jan 31, 2026 at 11:16:57AM -0500, Frank Li wrote:
> >On Sat, Jan 31, 2026 at 04:11:34PM +0800, Felix Gu wrote:
> >> The function pll6_bypassed() calls of_parse_phandle_with_args()
> >> but never calls of_node_put() to release the reference, causing
> >> a memory leak.
> >>
> >> Fix this by adding proper cleanup calls on all exit paths.
> >>
> >> Fixes: 3cc48976e9763 ("clk: imx6q: handle ENET PLL bypass")
> >> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> >> ---
> >>  drivers/clk/imx/clk-imx6q.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> >> index bf4c1d9c9928..1d8e8f0891a3 100644
> >> --- a/drivers/clk/imx/clk-imx6q.c
> >> +++ b/drivers/clk/imx/clk-imx6q.c
> >> @@ -238,8 +238,11 @@ static bool pll6_bypassed(struct device_node *node)
> >>                      return false;
> >>
>
> One more comment:
> Put one line here, "of_node_put", no need duplicate it in below.
Hi Peng,
Thanks for your review.
Frank also mentioned this in V1, but I think it may not be good to use
clkspec.np after calling of_node_put().

Best regards,
Felix Gu
>
> >>              if (clkspec.np == node &&
> >> -                clkspec.args[0] == IMX6QDL_PLL6_BYPASS)
> >> +                clkspec.args[0] == IMX6QDL_PLL6_BYPASS) {
> >> +                    of_node_put(clkspec.np);
>
> Drop this change
>
> >>                      break;
> >> +            }
> >> +            of_node_put(clkspec.np);
>
> Ditto.
>
> Regards,
> Peng
>
> >>      }
> >>
> >>      /* PLL6 bypass is not part of the assigned clock list */
> >> @@ -249,6 +252,7 @@ static bool pll6_bypassed(struct device_node *node)
> >>      ret = of_parse_phandle_with_args(node, "assigned-clock-parents",
> >>                                       "#clock-cells", index, &clkspec);
> >>
> >
> >If ret is err, clkspec will not touched. So clkspec.np keep old value,
> >of_node_put(clkspec.np) will put twice for previous np.
> >
> >So need add if (ret) check here. Or use difference variable clkspec with
> >init 0.
> >
> >Frank
> >
> >> +    of_node_put(clkspec.np);
> >>      if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
> >>              return true;
> >>
> >>
> >> --
> >> 2.43.0
> >>

Re: [PATCH v2 1/2] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()

Posted by Felix Gu 1 week, 1 day ago

On Sun, Feb 1, 2026 at 12:17 AM Frank Li <Frank.li@nxp.com> wrote:
>
> On Sat, Jan 31, 2026 at 04:11:34PM +0800, Felix Gu wrote:
> > The function pll6_bypassed() calls of_parse_phandle_with_args()
> > but never calls of_node_put() to release the reference, causing
> > a memory leak.
> >
> > Fix this by adding proper cleanup calls on all exit paths.
> >
> > Fixes: 3cc48976e9763 ("clk: imx6q: handle ENET PLL bypass")
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> > ---
> >  drivers/clk/imx/clk-imx6q.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> > index bf4c1d9c9928..1d8e8f0891a3 100644
> > --- a/drivers/clk/imx/clk-imx6q.c
> > +++ b/drivers/clk/imx/clk-imx6q.c
> > @@ -238,8 +238,11 @@ static bool pll6_bypassed(struct device_node *node)
> >                       return false;
> >
> >               if (clkspec.np == node &&
> > -                 clkspec.args[0] == IMX6QDL_PLL6_BYPASS)
> > +                 clkspec.args[0] == IMX6QDL_PLL6_BYPASS) {
> > +                     of_node_put(clkspec.np);
> >                       break;
> > +             }
> > +             of_node_put(clkspec.np);
> >       }
> >
> >       /* PLL6 bypass is not part of the assigned clock list */
> > @@ -249,6 +252,7 @@ static bool pll6_bypassed(struct device_node *node)
> >       ret = of_parse_phandle_with_args(node, "assigned-clock-parents",
> >                                        "#clock-cells", index, &clkspec);
> >
>
> If ret is err, clkspec will not touched. So clkspec.np keep old value,
> of_node_put(clkspec.np) will put twice for previous np.
>
> So need add if (ret) check here. Or use difference variable clkspec with
> init 0.
Thanks, I will fix it in v3.

>
> Frank
>
> > +     of_node_put(clkspec.np);
> >       if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
> >               return true;
> >
> >
> > --
> > 2.43.0
> >

Best regards,
Felix Gu