Re: [PATCH] staging: greybus: light: fix leak of cdev->name

[Nguyễn Công]

Hi Greg,
I used an LLM-based agent (Claude) to flag allocations that aren't freed
on error paths, then I verified this one by hand: I traced every place
cdev->name (from kasprintf()) is freed and found that
__gb_lights_led_unregister() misses it on the config-failure and flash
channel teardown paths. No static-analysis tool was used - just the
agent to surface the candidate plus manual review.

thanks,
Cong

On Tue, Jul 28, 2026 at 2:17 PM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, Jul 26, 2026 at 06:00:39PM +0700, Cong Nguyen wrote:
> > gb_lights_channel_config() builds the LED classdev name with kasprintf()
> > and stores it in cdev->name for every channel during the configuration
> > phase, which runs before the channel is registered.
> >
> > That name was only released by __gb_lights_led_unregister(), i.e. only for
> > a normal LED channel that was actually registered. It was therefore leaked
> > in two cases:
> >
> >   - a channel that is configured but never registered, e.g.
> >     channel_attr_groups_set() fails, the flash configuration fails, or a
> >     later channel in the same light fails to configure and the whole light
> >     is torn down; the release path calls gb_lights_channel_unregister()
> >     (which returns early because the channel is not registered) and then
> >     gb_lights_channel_free();
> >
> >   - a flash, torch or indicator channel, whose unregister path
> >     (__gb_lights_flash_led_unregister()) never releases cdev->name at all,
> >     so the name leaks even on a successful teardown.
> >
> > cdev->name is a configuration-phase allocation, like channel->color_name
> > and channel->mode_name. Free it in gb_lights_channel_free() next to
> > those, since that runs unconditionally on every teardown path, and drop
> > the special case free from __gb_lights_led_unregister().
> > gb_lights_channel_unregister() is only ever called from
> > gb_lights_channel_release(), immediately followed by
> > gb_lights_channel_free(), so the name is still released on the registered
> > path and is freed exactly once.
> >
> > Commit 04820da21050 ("staging: greybus: light: Release memory obtained by
> > kasprintf") fixed the leak for the registered normal-LED path only; this
> > covers the remaining cases.
> >
> > Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> > Signed-off-by: Cong Nguyen <congnt264@xxxxxxxxx>
> > ---
> >  drivers/staging/greybus/light.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
>
> Cool, but how did you find this issue?  What tool did you use?
>
> thanks,
>
> greg k-h