[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAN4SLj3-0NH232gauZs1+HpL2A0oE-QTh8L8BYgjKAwNVFYi0w@mail.gmail.com>
Date: Sat, 31 Jan 2026 02:00:54 +0800
From: Felix Gu <ustc.gu@...il.com>
To: Frank Li <Frank.li@....com>
Cc: Abel Vesa <abelvesa@...nel.org>, Peng Fan <peng.fan@....com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, Lucas Stach <l.stach@...gutronix.de>, linux-clk@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: imx: imx6q: Fix device node reference leak in pll6_bypassed()
On Sat, Jan 31, 2026 at 1:37 AM Frank Li <Frank.li@....com> wrote:
>
> On Fri, Jan 30, 2026 at 11:56:28PM +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@...il.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);
>
>
> I think you put of_node_put() before if branch
I didn't put of_node_put() before if branch is clksepc.np is used in
the if branch.
It maybe not good to use it after calling of_node_put().
>
> rc = of_parse_phandle_with_args()
> if (rc < 0)
> ...
> of_node_put(clkspec.np);
>
>
> There are more places call of_parse_phandle_with_args() without of_node_put().
Thanks, I missed that, I will fix it in V2.
Best regards,
Felix Gu
>
>
> > }
> >
> > /* 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);
> >
>
> this one miss check ret;
>
> Frank
> > + of_node_put(clkspec.np);
> > if (clkspec.args[0] != IMX6QDL_CLK_PLL6)
> > return true;
> >
> >
> > ---
> > base-commit: 33a647c659ffa5bdb94abc345c8c86768ff96215
> > change-id: 20260130-clk-imx6q-9c4b8835fc26
> >
> > Best regards,
> > --
> > Felix Gu <ustc.gu@...il.com>
> >
Powered by blists - more mailing lists