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@xxxxxxxxxxx> 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