All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Masney <bmasney@redhat.com>
To: Guangshuo Li <lgs201920130244@gmail.com>
Cc: Emil Renner Berthing <kernel@esmil.dk>,
	Hal Feng <hal.feng@starfivetech.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Conor Dooley <conor.dooley@microchip.com>,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path
Date: Mon, 13 Apr 2026 12:46:29 -0400	[thread overview]
Message-ID: <ad0d5fIAkjblQcIt@redhat.com> (raw)
In-Reply-To: <20260413143643.3002454-1-lgs201920130244@gmail.com>

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


  reply	other threads:[~2026-04-13 16:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-13 14:36 [PATCH v3] clk: starfive: jh7110: fix memory leak in jh7110_reset_controller_register() error path Guangshuo Li
2026-04-13 16:46 ` Brian Masney [this message]
2026-04-14 11:44   ` Guangshuo Li
2026-04-14 20:05     ` Brian Masney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad0d5fIAkjblQcIt@redhat.com \
    --to=bmasney@redhat.com \
    --cc=conor.dooley@microchip.com \
    --cc=hal.feng@starfivetech.com \
    --cc=kernel@esmil.dk \
    --cc=lgs201920130244@gmail.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.