public inbox for linux-clk@vger.kernel.org
 help / color / mirror / Atom feed
From: "Théo Lebrun" <theo.lebrun@bootlin.com>
To: "Guangshuo Li" <lgs201920130244@gmail.com>,
	"Vladimir Kondratiev" <vladimir.kondratiev@mobileye.com>,
	"Gregory CLEMENT" <gregory.clement@bootlin.com>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>, <linux-mips@vger.kernel.org>,
	<linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <stable@vger.kernel.org>
Subject: Re: [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path
Date: Mon, 13 Apr 2026 10:04:27 +0200	[thread overview]
Message-ID: <DHRVDETB559R.1J1MUGSZ0VVEX@bootlin.com> (raw)
In-Reply-To: <20260412124247.2494971-1-lgs201920130244@gmail.com>

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


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

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-12 12:42 [PATCH v2] clk: eyeq: fix memory leak in eqc_auxdev_create() error path Guangshuo Li
2026-04-13  8:04 ` Théo Lebrun [this message]
2026-04-14 11:51   ` Guangshuo Li
2026-04-13 16:42 ` Brian Masney
2026-04-14 11:49   ` Guangshuo Li
2026-04-14 20:06     ` 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=DHRVDETB559R.1J1MUGSZ0VVEX@bootlin.com \
    --to=theo.lebrun@bootlin.com \
    --cc=gregory.clement@bootlin.com \
    --cc=lgs201920130244@gmail.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=vladimir.kondratiev@mobileye.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox