Re: [PATCH v2] clk: spacemit: fix resource leak in spacemit_ccu_reset_register

[Alex Elder]

On 7/26/25 4:16 AM, Christophe JAILLET wrote:
> Le 23/07/2025 à 15:25, Hendrik Hamerlinck a écrit :
> > The function spacemit_ccu_reset_register() allocates memory for an
> > auxiliary device. If auxiliary_device_add() fails, it skips cleanup of
> > these resources, resulting in leaks.
> >
> > Fix this by using the appropriate error handling path.
> >
> > Fixes: 988543522ebd ("clk: spacemit: set up reset auxiliary devices")
> > Signed-off-by: Hendrik Hamerlinck <hendrik.hamerlinck@xxxxxxxxxxxx>
> > Reviewed-by: Yixun Lan <dlan@xxxxxxxxxx>
> > ---
> > Changes in v2:
> > - Properly place the Fixes tip.
> > ---
> >   drivers/clk/spacemit/ccu-k1.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu- 
> > k1.c
> > index 65e6de030717..5bb85e32c6cf 100644
> > --- a/drivers/clk/spacemit/ccu-k1.c
> > +++ b/drivers/clk/spacemit/ccu-k1.c
> > @@ -1059,7 +1059,7 @@ static int spacemit_ccu_reset_register(struct 
> > device *dev,
> >       ret = auxiliary_device_add(adev);
> >       if (ret) {
> >           auxiliary_device_uninit(adev);
> > -        return ret;
> > +        goto err_free_aux_id;
> >       }
> >       return devm_add_action_or_reset(dev, spacemit_adev_unregister, 
> > adev);
>
>
> Hi,
>
> I'm not sure this is correct.

I'm sure this patch is incorrect, and the original code did
not have the stated problem.  Thank you for calling attention
to this Christophe.

Stephen, could you please revert your commit of this patch?

Thank you.

					-Alex

> auxiliary_device_uninit() which is called if auxiliary_device_add() 
> fails should already do the clean-up.
>
> So I would say that this patch adds a double-free issue and should not 
> be applied.
>
> If I'm right, note that Stephen has already added a "Applied to clk-next"
>
> CJ
>
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv