[PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path

[PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path

[Guangshuo Li]

jh7110_reset_controller_register() allocates a jh71x0_reset_adev with
kzalloc() and sets jh7110_reset_adev_release() as the release callback
for its embedded auxiliary_device before calling auxiliary_device_init().

If auxiliary_device_init() fails, the function returns immediately
without freeing the allocated rdev. The release callback is not
available for this path, because it is only reached after a successful
auxiliary_device_init(), for example when auxiliary_device_add() fails
and auxiliary_device_uninit() is called.

The issue was identified by a static analysis tool I developed and
confirmed by manual review. Free rdev explicitly when
auxiliary_device_init() returns an error.

Fixes: edab7204afe5 ("clk: starfive: Add StarFive JH7110 system clock driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v3:
  - clarify the changelog to describe the exact failure path
  - note that the issue was identified by a static analysis tool
    developed by me and confirmed by manual review
  - apologize for sending the initial public posting as v2 by mistake

v2:
  - initial public posting; v1 was mistakenly skipped

 drivers/clk/starfive/clk-starfive-jh7110-sys.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
index 52833d4241c5..55cd0ccbdb84 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
@@ -360,8 +360,10 @@ int jh7110_reset_controller_register(struct jh71x0_clk_priv *priv,
 	adev->id = adev_id;
 
 	ret = auxiliary_device_init(adev);
-	if (ret)
+	if (ret) {
+		kfree(rdev);
 		return ret;
+	}
 
 	ret = auxiliary_device_add(adev);
 	if (ret) {
-- 
2.43.0

Re: [PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path

[Brian Masney]

Hi Guangshuo,

I missed that you sent a new version. My same comment from the v2 still
applies. See below for details.

On Mon, Apr 13, 2026 at 10:36:43PM +0800, Guangshuo Li wrote:
> jh7110_reset_controller_register() allocates a jh71x0_reset_adev with
> kzalloc() and sets jh7110_reset_adev_release() as the release callback
> for its embedded auxiliary_device before calling auxiliary_device_init().
> 
> If auxiliary_device_init() fails, the function returns immediately
> without freeing the allocated rdev. The release callback is not
> available for this path, because it is only reached after a successful
> auxiliary_device_init(), for example when auxiliary_device_add() fails
> and auxiliary_device_uninit() is called.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review. Free rdev explicitly when
> auxiliary_device_init() returns an error.
> 
> Fixes: edab7204afe5 ("clk: starfive: Add StarFive JH7110 system clock driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v3:
>   - clarify the changelog to describe the exact failure path
>   - note that the issue was identified by a static analysis tool
>     developed by me and confirmed by manual review
>   - apologize for sending the initial public posting as v2 by mistake
> 
> v2:
>   - initial public posting; v1 was mistakenly skipped
> 
>  drivers/clk/starfive/clk-starfive-jh7110-sys.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> index 52833d4241c5..55cd0ccbdb84 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> @@ -360,8 +360,10 @@ int jh7110_reset_controller_register(struct jh71x0_clk_priv *priv,
>  	adev->id = adev_id;
>  
>  	ret = auxiliary_device_init(adev);
> -	if (ret)
> +	if (ret) {
> +		kfree(rdev);
>  		return ret;
> +	}
>  
>  	ret = auxiliary_device_add(adev);
>  	if (ret) {

There's actually another leak in the error path for
auxiliary_device_add(). I think this code should be
converted to devm_kzalloc().

There is no devm_kzalloc_obj() yet, however according to [1] that should
be coming soon.

[1] https://lore.kernel.org/lkml/20260330154108.GA3389518@killaraus.ideasonboard.com/

Brian

Re: [PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path

[Guangshuo Li]

Hi Brian,

Thanks for reviewing.

On Tue, 14 Apr 2026 at 00:46, Brian Masney <bmasney@redhat.com> wrote:

> There's actually another leak in the error path for
> auxiliary_device_add(). I think this code should be
> converted to devm_kzalloc().
>
> There is no devm_kzalloc_obj() yet, however according to [1] that should
> be coming soon.
>
> [1] https://lore.kernel.org/lkml/20260330154108.GA3389518@killaraus.ideasonboard.com/
>
> Brian
>

I may be missing something, but I think the auxiliary_device_add() error
path is already handled here:

ret = auxiliary_device_add(adev);
if (ret) {
        auxiliary_device_uninit(adev);
        return ret;
}

The embedded auxiliary_device has:

adev->dev.release = jh7110_reset_adev_release;

and the release callback does:

static void jh7110_reset_adev_release(struct device *dev)
{
        struct auxiliary_device *adev = to_auxiliary_dev(dev);
        struct jh71x0_reset_adev *rdev = to_jh71x0_reset_adev(adev);

        kfree(rdev);
}

So my understanding was that after a successful auxiliary_device_init(),
the auxiliary_device_add() failure path should be cleaned up through
auxiliary_device_uninit(), which would eventually call the release
callback and free rdev.

The leak I was trying to fix is only the auxiliary_device_init() failure
path, where the function returns directly before that cleanup mechanism is
available.

Please let me know if I overlooked something.

Thanks,
Guangshuo

Re: [PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path

[Brian Masney]

On Tue, Apr 14, 2026 at 07:44:18PM +0800, Guangshuo Li wrote:
> Hi Brian,
> 
> Thanks for reviewing.
> 
> On Tue, 14 Apr 2026 at 00:46, Brian Masney <bmasney@redhat.com> wrote:
> 
> > There's actually another leak in the error path for
> > auxiliary_device_add(). I think this code should be
> > converted to devm_kzalloc().
> >
> > There is no devm_kzalloc_obj() yet, however according to [1] that should
> > be coming soon.
> >
> > [1] https://lore.kernel.org/lkml/20260330154108.GA3389518@killaraus.ideasonboard.com/
> >
> > Brian
> >
> 
> I may be missing something, but I think the auxiliary_device_add() error
> path is already handled here:
> 
> ret = auxiliary_device_add(adev);
> if (ret) {
>         auxiliary_device_uninit(adev);
>         return ret;
> }
> 
> The embedded auxiliary_device has:
> 
> adev->dev.release = jh7110_reset_adev_release;
> 
> and the release callback does:

You are right. Sorry about that. My original suggestion still applies
though to move over to the devm variant since that'll allow you to
remove the release callback.

Brian