[PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Guangshuo Li]

eqc_auxdev_create() allocates an auxiliary_device with kzalloc() before
calling auxiliary_device_init().

When auxiliary_device_init() returns an error, the function exits
without freeing adev. Since the release callback is only expected to
handle cleanup after successful initialization, adev should be freed
explicitly in this path.

Add the missing kfree(adev) before returning from the
auxiliary_device_init() error path.

Fixes: 25d904946a0b ("clk: eyeq: add driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/clk/clk-eyeq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
index ea1c3d78e7cd..a48ecec4c9a5 100644
--- a/drivers/clk/clk-eyeq.c
+++ b/drivers/clk/clk-eyeq.c
@@ -346,8 +346,10 @@ static int eqc_auxdev_create(struct device *dev, void __iomem *base,
 	adev->id = id;
 
 	ret = auxiliary_device_init(adev);
-	if (ret)
+	if (ret) {
+		kfree(adev);
 		return ret;
+	}
 
 	ret = auxiliary_device_add(adev);
 	if (ret)
-- 
2.43.0

Re: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Théo Lebrun]

Hello Guangshuo,

Subject is:

> Subject: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create()
>          error path

I cannot find a public V1?
https://lore.kernel.org/lkml/?q=s%3Aeyeq+f%3AGuangshuo

On Sun Apr 12, 2026 at 2:42 PM CEST, Guangshuo Li wrote:
> eqc_auxdev_create() allocates an auxiliary_device with kzalloc() before
> calling auxiliary_device_init().
>
> When auxiliary_device_init() returns an error, the function exits
> without freeing adev. Since the release callback is only expected to
> handle cleanup after successful initialization, adev should be freed
> explicitly in this path.
>
> Add the missing kfree(adev) before returning from the
> auxiliary_device_init() error path.
>
> Fixes: 25d904946a0b ("clk: eyeq: add driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

I have a guess this is LLM generated?
Are you missing the Assisted-by trailer?
https://docs.kernel.org/process/coding-assistants.html#attribution

The patch could be in theory useful.
In practice however, it's a different story.

 - Comit message says "Since the release callback is only expected to
   handle cleanup after successful initialization, adev should be freed
   explicitly in this path".

   Two things are wrong here:

   1. the driver cannot be removed so there is no "release
      callback" (guessing you mean driver remove?).

   2. this text seems to imply eqc_auxdev_create() makes probe fail,
      which it doesn't. It only generates a warning and keeps probing.

 - This driver cannot be built as module (will always be probed at boot)
   and cannot be removed. So the "leak" we are talking about is
   2 * sizeof(struct auxiliary_device)

   But in no sensible case it can occur. The platforms that use this
   driver probably cannot boot if our auxiliary drivers aren't present.
   So if eqc_auxdev_create() fails then the warning is here to be nice
   but you probably will fail booting afterwards.

   My guess is: you might succeed booting without the reset driver but
   if you fail the pinctrl one then you won't have a UART. Anyway in no
   world do you have a sensible EyeQ kernel config that leads to
   clk-eyeq probing but not its auxdevs.

 - If you fix this then there are other resources cleanup to "fix".

    - ioremap() in eqc_probe()
    - kzalloc of cells in eqc_probe()
    - probably others

   But, same as above, "fixing" those will only be useful in kernels
   that will panic in a few milliseconds.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Brian Masney]

Hi Guangshuo,

On Sun, Apr 12, 2026 at 08:42:46PM +0800, Guangshuo Li wrote:
> eqc_auxdev_create() allocates an auxiliary_device with kzalloc() before
> calling auxiliary_device_init().
> 
> When auxiliary_device_init() returns an error, the function exits
> without freeing adev. Since the release callback is only expected to
> handle cleanup after successful initialization, adev should be freed
> explicitly in this path.
> 
> Add the missing kfree(adev) before returning from the
> auxiliary_device_init() error path.
> 
> Fixes: 25d904946a0b ("clk: eyeq: add driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/clk/clk-eyeq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> index ea1c3d78e7cd..a48ecec4c9a5 100644
> --- a/drivers/clk/clk-eyeq.c
> +++ b/drivers/clk/clk-eyeq.c
> @@ -346,8 +346,10 @@ static int eqc_auxdev_create(struct device *dev, void __iomem *base,
>  	adev->id = id;
>  
>  	ret = auxiliary_device_init(adev);
> -	if (ret)
> +	if (ret) {
> +		kfree(adev);
>  		return ret;
> +	}
>  
>  	ret = auxiliary_device_add(adev);
>  	if (ret)

There is a leak in the error path here as well. 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 v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Guangshuo Li]

Hi Brian,

Thanks for reviewing.

On Tue, 14 Apr 2026 at 00:42, Brian Masney <bmasney@redhat.com> wrote:
>
> There is a leak in the error path here as well. 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);

The auxiliary device also has:

adev->dev.release = eqc_auxdev_release;

with:

static void eqc_auxdev_release(struct device *dev)
{
        struct auxiliary_device *adev = to_auxiliary_dev(dev);

        kfree(adev);
}

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 invoke the release
callback and free adev.

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

Please let me know if I overlooked something.

Thanks,
Guangshuo

Re: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Guangshuo Li]

Hi Théo,

Thank you for the review, and sorry for the confusion here.

On Mon, 13 Apr 2026 at 16:04, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>

> Subject is:
>
> > Subject: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create()
> >          error path
>
> I cannot find a public V1?
> https://lore.kernel.org/lkml/?q=s%3Aeyeq+f%3AGuangshuo

This was my mistake: I did not send a public V1 and sent this directly as V2.

> I have a guess this is LLM generated?
> Are you missing the Assisted-by trailer?
> https://docs.kernel.org/process/coding-assistants.html#attribution

This issue was found by a static analysis tool I developed.

That said, the changelog was clearly too brief and did not explain the
actual situation well enough, which likely caused the confusion.

> The patch could be in theory useful.
> In practice however, it's a different story.
>
>  - Comit message says "Since the release callback is only expected to
>    handle cleanup after successful initialization, adev should be freed
>    explicitly in this path".
>
>    Two things are wrong here:
>
>    1. the driver cannot be removed so there is no "release
>       callback" (guessing you mean driver remove?).
>
>    2. this text seems to imply eqc_auxdev_create() makes probe fail,
>       which it doesn't. It only generates a warning and keeps probing.
>
>  - This driver cannot be built as module (will always be probed at boot)
>    and cannot be removed. So the "leak" we are talking about is
>    2 * sizeof(struct auxiliary_device)
>
>    But in no sensible case it can occur. The platforms that use this
>    driver probably cannot boot if our auxiliary drivers aren't present.
>    So if eqc_auxdev_create() fails then the warning is here to be nice
>    but you probably will fail booting afterwards.
>
>    My guess is: you might succeed booting without the reset driver but
>    if you fail the pinctrl one then you won't have a UART. Anyway in no
>    world do you have a sensible EyeQ kernel config that leads to
>    clk-eyeq probing but not its auxdevs.
>
>  - If you fix this then there are other resources cleanup to "fix".
>
>     - ioremap() in eqc_probe()
>     - kzalloc of cells in eqc_probe()
>     - probably others
>
>    But, same as above, "fixing" those will only be useful in kernels
>    that will panic in a few milliseconds.
>
What I intended to express with:

"Since the release callback is only expected to handle cleanup after
successful initialization, adev should be freed explicitly in this
path"

was more specifically this:

eqc_auxdev_release() is the callback that eventually frees adev, but
this path is only reachable after auxiliary_device_init() has
succeeded. For example, when auxiliary_device_add() fails,
auxiliary_device_uninit() can lead to that release path. By contrast,
if auxiliary_device_init() itself fails, the function returns
directly, and adev is not explicitly freed in that path.

So the point I meant to describe was only the missing explicit free in
the auxiliary_device_init() failure path.

Sorry again for the misunderstanding caused by the previous changelog.

Thanks,
Guangshuo

Re: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path

[Brian Masney]

On Tue, Apr 14, 2026 at 07:49:31PM +0800, Guangshuo Li wrote:
> Hi Brian,
> 
> Thanks for reviewing.
> 
> On Tue, 14 Apr 2026 at 00:42, Brian Masney <bmasney@redhat.com> wrote:
> >
> > There is a leak in the error path here as well. 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);
> 
> The auxiliary device also has:
> 
> adev->dev.release = eqc_auxdev_release;
> 
> with:
> 
> static void eqc_auxdev_release(struct device *dev)
> {
>         struct auxiliary_device *adev = to_auxiliary_dev(dev);
> 
>         kfree(adev);
> }
> 
> 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 invoke the release
> callback and free adev.
> 
> The leak I was trying to fix is only the auxiliary_device_init() failure
> path, where the function returns directly before that cleanup path is
> available.
> 
> Please let me know if I overlooked something.

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