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")
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264(a)gmail.com>
---
Changes in v2:
- Add Assisted-by: tag to document AI assistance, per
Documentation/process/coding-assistants.rst (Greg KH).
- No functional change.
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
In gb_bootrom_get_firmware(), when fw is NULL, ret is already set to
ERRNO before jumping to the queue_work label, so !ret is always
false and fw->size is never dereferenced. Add an explicit NULL check
to silence a coccinelle false positive warning.
Signed-off-by: Pei Xiao <xiaopei01(a)kylinos.cn>
---
drivers/staging/greybus/bootrom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
index 83921d90c322..50c80475d241 100644
--- a/drivers/staging/greybus/bootrom.c
+++ b/drivers/staging/greybus/bootrom.c
@@ -298,7 +298,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
queue_work:
/* Refresh timeout */
- if (!ret && (offset + size == fw->size))
+ if (!ret && fw && (offset + size == fw->size))
next_request = NEXT_REQ_READY_TO_BOOT;
else
next_request = NEXT_REQ_GET_FIRMWARE;
--
2.25.1
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(a)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