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

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

[Cong Nguyen]

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@gmail.com>
---
 drivers/staging/greybus/light.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index cab02b5da867..1ecca479b5f4 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -905,8 +905,6 @@ static void __gb_lights_led_unregister(struct gb_channel *channel)
 		return;
 
 	led_classdev_unregister(cdev);
-	kfree(cdev->name);
-	cdev->name = NULL;
 	channel->led = NULL;
 }
 
@@ -1063,6 +1061,9 @@ static int gb_lights_light_register(struct gb_light *light)
 
 static void gb_lights_channel_free(struct gb_channel *channel)
 {
+	struct led_classdev *cdev = get_channel_cdev(channel);
+
+	kfree(cdev->name);
 	kfree(channel->attrs);
 	kfree(channel->attr_group);
 	kfree(channel->attr_groups);
-- 
2.25.1

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

[Greg Kroah-Hartman]

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@gmail.com>
> ---
>  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