[PATCH] staging: greybus: lights: avoid NULL deref

[PATCH] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

gb_lights_light_config() stores channel_count before allocating the
channels array. If kcalloc() fails, gb_lights_release() iterates the
non-zero count and dereferences light->channels, which is NULL.

Allocate channels first and only then publish channels_count so the
cleanup path can't walk a NULL pointer.

Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
---
 drivers/staging/greybus/light.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index e509fdc715db..4c9ad9ea8827 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -1008,14 +1008,14 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
 	if (!strlen(conf.name))
 		return -EINVAL;
 
-	light->channels_count = conf.channel_count;
 	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
 	if (!light->name)
 		return -ENOMEM;
-	light->channels = kcalloc(light->channels_count,
+	light->channels = kcalloc(conf.channel_count,
 				  sizeof(struct gb_channel), GFP_KERNEL);
 	if (!light->channels)
 		return -ENOMEM;
+	light->channels_count = conf.channel_count;
 
 	/* First we collect all the configurations for all channels */
 	for (i = 0; i < light->channels_count; i++) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] staging: greybus: lights: avoid NULL deref

[Greg KH]

On Thu, Jan 08, 2026 at 04:07:00PM +0530, Chaitanya Mishra wrote:
> gb_lights_light_config() stores channel_count before allocating the
> channels array. If kcalloc() fails, gb_lights_release() iterates the
> non-zero count and dereferences light->channels, which is NULL.
> 
> Allocate channels first and only then publish channels_count so the
> cleanup path can't walk a NULL pointer.

How was this issue found?  How was the fix generated?  How was it
tested?

> 
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
> ---
>  drivers/staging/greybus/light.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index e509fdc715db..4c9ad9ea8827 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1008,14 +1008,14 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
>  	if (!strlen(conf.name))
>  		return -EINVAL;
>  
> -	light->channels_count = conf.channel_count;
>  	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
>  	if (!light->name)
>  		return -ENOMEM;
> -	light->channels = kcalloc(light->channels_count,
> +	light->channels = kcalloc(conf.channel_count,
>  				  sizeof(struct gb_channel), GFP_KERNEL);
>  	if (!light->channels)
>  		return -ENOMEM;
> +	light->channels_count = conf.channel_count;

This is "tricky", perhaps add a comment here as to why you are only
assigning this now and not before?

thanks,

greg k-h

[PATCH v2] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

gb_lights_light_config() stores channel_count before allocating the
channels array. If kcalloc() fails, gb_lights_release() iterates the
non-zero count and dereferences light->channels, which is NULL.

Allocate channels first and only then publish channels_count so the
cleanup path can't walk a NULL pointer.

Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
---
 drivers/staging/greybus/light.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index e509fdc715db..38c233a706c4 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -1008,14 +1008,18 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
 	if (!strlen(conf.name))
 		return -EINVAL;
 
-	light->channels_count = conf.channel_count;
 	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
 	if (!light->name)
 		return -ENOMEM;
-	light->channels = kcalloc(light->channels_count,
+	light->channels = kcalloc(conf.channel_count,
 				  sizeof(struct gb_channel), GFP_KERNEL);
 	if (!light->channels)
 		return -ENOMEM;
+	/*
+	 * Publish channels_count only after channels allocation so cleanup
+	 * doesn't walk a NULL channels pointer on allocation failure.
+	 */
+	light->channels_count = conf.channel_count;
 
 	/* First we collect all the configurations for all channels */
 	for (i = 0; i < light->channels_count; i++) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] staging: greybus: lights: avoid NULL deref

[Greg KH]

On Thu, Jan 08, 2026 at 04:19:47PM +0530, Chaitanya Mishra wrote:
> gb_lights_light_config() stores channel_count before allocating the
> channels array. If kcalloc() fails, gb_lights_release() iterates the
> non-zero count and dereferences light->channels, which is NULL.
> 
> Allocate channels first and only then publish channels_count so the
> cleanup path can't walk a NULL pointer.
> 
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
> ---
>  drivers/staging/greybus/light.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index e509fdc715db..38c233a706c4 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1008,14 +1008,18 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
>  	if (!strlen(conf.name))
>  		return -EINVAL;
>  
> -	light->channels_count = conf.channel_count;
>  	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
>  	if (!light->name)
>  		return -ENOMEM;
> -	light->channels = kcalloc(light->channels_count,
> +	light->channels = kcalloc(conf.channel_count,
>  				  sizeof(struct gb_channel), GFP_KERNEL);
>  	if (!light->channels)
>  		return -ENOMEM;
> +	/*
> +	 * Publish channels_count only after channels allocation so cleanup
> +	 * doesn't walk a NULL channels pointer on allocation failure.
> +	 */
> +	light->channels_count = conf.channel_count;
>  
>  	/* First we collect all the configurations for all channels */
>  	for (i = 0; i < light->channels_count; i++) {
> -- 
> 2.50.1 (Apple Git-155)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v2] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

Hi Greg,

Found by manual code review while walking the error paths in
Gb_lights_light_config(): channels_count is set before channels
allocation, but cleanup uses channels_count to iterate and dereference
light->channels. If kcalloc() fails, that becomes a NULL deref.

Fix is simply deferring channels_count publication until after the
allocation succeeds; v2 includes the requested comment.

Tested with:
  ./scripts/checkpatch.pl --strict -g HEAD
  ./scripts/checkpatch.pl outgoing/0001-staging-greybus-lights-avoid-NULL-deref.patch

I couldn't build-test locally on macOS due to missing <elf.h> for
kernel host tools.

Thanks,
Chaitanya

Re: [PATCH v2] staging: greybus: lights: avoid NULL deref

[Greg KH]

On Thu, Jan 08, 2026 at 04:33:51PM +0530, Chaitanya Mishra wrote:
> Hi Greg,
> 
> Found by manual code review while walking the error paths in
> Gb_lights_light_config(): channels_count is set before channels
> allocation, but cleanup uses channels_count to iterate and dereference
> light->channels. If kcalloc() fails, that becomes a NULL deref.

Might I ask why are you manually reviewing the error code paths of this
driver?  Do you have this hardware somewhere?

> Fix is simply deferring channels_count publication until after the
> allocation succeeds; v2 includes the requested comment.
> 
> Tested with:
>   ./scripts/checkpatch.pl --strict -g HEAD
>   ./scripts/checkpatch.pl outgoing/0001-staging-greybus-lights-avoid-NULL-deref.patch
> 
> I couldn't build-test locally on macOS due to missing <elf.h> for
> kernel host tools.

For obvious reasons, sending out patches that you didn't even build test
is probably not a good idea :)

thanks,

greg k-h

Re: [PATCH v2] staging: greybus: lights: avoid NULL deref

[Rui Miguel Silva]

Hey Chaitanya,
Thanks for the patch.

On Thu Jan 8, 2026 at 10:49 AM WET, Chaitanya Mishra wrote:

> gb_lights_light_config() stores channel_count before allocating the
> channels array. If kcalloc() fails, gb_lights_release() iterates the
> non-zero count and dereferences light->channels, which is NULL.
>
> Allocate channels first and only then publish channels_count so the
> cleanup path can't walk a NULL pointer.

Good catch, going through the error path, does looks this fix the issue.

But you need to add the changes between versions bellow --  and maybe a
link to the first version.

Reviewed-by: Rui Miguel Silva <rui.silva@linaro.org>

Cheers,
    Rui
>
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
> ---
>  drivers/staging/greybus/light.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index e509fdc715db..38c233a706c4 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1008,14 +1008,18 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
>  	if (!strlen(conf.name))
>  		return -EINVAL;
>  
> -	light->channels_count = conf.channel_count;
>  	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
>  	if (!light->name)
>  		return -ENOMEM;
> -	light->channels = kcalloc(light->channels_count,
> +	light->channels = kcalloc(conf.channel_count,
>  				  sizeof(struct gb_channel), GFP_KERNEL);
>  	if (!light->channels)
>  		return -ENOMEM;
> +	/*
> +	 * Publish channels_count only after channels allocation so cleanup
> +	 * doesn't walk a NULL channels pointer on allocation failure.
> +	 */
> +	light->channels_count = conf.channel_count;
>  
>  	/* First we collect all the configurations for all channels */
>  	for (i = 0; i < light->channels_count; i++) {
> -- 
> 2.50.1 (Apple Git-155)

[PATCH v3] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

gb_lights_light_config() stores channel_count before allocating the
channels array. If kcalloc() fails, gb_lights_release() iterates the
non-zero count and dereferences light->channels, which is NULL.

Allocate channels first and only then publish channels_count so the
cleanup path can't walk a NULL pointer.

Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
Signed-off-by: Chaitanya Mishra <chaitanyamishra.ai@gmail.com>
---
Changes in v3:
- Add version changelog below the --- line (no code changes).

 drivers/staging/greybus/light.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index e509fdc715db..38c233a706c4 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -1008,14 +1008,18 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
 	if (!strlen(conf.name))
 		return -EINVAL;
 
-	light->channels_count = conf.channel_count;
 	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
 	if (!light->name)
 		return -ENOMEM;
-	light->channels = kcalloc(light->channels_count,
+	light->channels = kcalloc(conf.channel_count,
 				  sizeof(struct gb_channel), GFP_KERNEL);
 	if (!light->channels)
 		return -ENOMEM;
+	/*
+	 * Publish channels_count only after channels allocation so cleanup
+	 * doesn't walk a NULL channels pointer on allocation failure.
+	 */
+	light->channels_count = conf.channel_count;
 
 	/* First we collect all the configurations for all channels */
 	for (i = 0; i < light->channels_count; i++) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

Hi Rui,

Thanks for the review.

I sent v3 with the version history below the --- line (no code changes).
I couldn't add a lore link yet since the first version doesn't appear to
be indexed; if it shows up and a respin is needed, I can add a Link: tag
and carry your Reviewed-by.

Thanks,
Chaitanya

Re: [PATCH v3] staging: greybus: lights: avoid NULL deref

[Greg KH]

On Thu, Jan 08, 2026 at 04:39:26PM +0530, Chaitanya Mishra wrote:
> Hi Rui,
> 
> Thanks for the review.
> 
> I sent v3 with the version history below the --- line (no code changes).
> I couldn't add a lore link yet since the first version doesn't appear to
> be indexed;

I see it there:
	https://lore.kernel.org/all/20260108103700.15384-1-chaitanyamishra.ai@gmail.com/

> if it shows up and a respin is needed, I can add a Link: tag
> and carry your Reviewed-by.

You should have added it from v2.

Also, slow down, there's no rush here.  Usually you should wait at least
a day between new versions, right?

thanks,

greg k-h

Re: [PATCH v3] staging: greybus: lights: avoid NULL deref

[Chaitanya Mishra]

Hi Greg,

You're right on both points. I don't have the hardware; I was walking
error paths in staging drivers to learn and look for obvious bugs, but
I shouldn't have sent a patch without a build test. Sorry for the noise.

I'll set up a Linux build environment, rerun at least a module build
with checkpatch, and wait before posting any v4. If a respin is still
needed, I'll add a Link: tag to v1, include Rui's Reviewed-by, and keep
a proper changelog below the --- line. Please ignore v3 for now.

Thanks,
Chaitanya