lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 2 Mar 2024 09:31:38 -0600
From: Alex Elder <elder@...e.org>
To: Mikhail Lobanov <m.lobanov@...alinux.ru>,
 Rui Miguel Silva <rmfrfs@...il.com>
Cc: greybus-dev@...ts.linaro.org, linux-staging@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Subject: Re: [greybus-dev] [PATCH] greybus: Fix deref of NULL in
 __gb_lights_flash_brightness_set

On 3/1/24 1:04 PM, Mikhail Lobanov wrote:
> Dereference of null pointer in the __gb_lights_flash_brightness_set function.
> Assigning the channel the result of executing the get_channel_from_mode function
> without checking for NULL may result in an error.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.

I think this is an actual problem but this might not be the
right fix.

The point of the call to get_channel_from_mode() is to get
the attached torch channel if the passed-in channel is a
flash channel.  It's *possible* that any flash channel will
*always* have an attached torch channel, but if so there
ought to be a comment to that effect near this call (to
explain why there's no need for the null pointer check).

I think Dan's suggestion should be implemented as well.
It's possible the intention really *was* to have
get_channel_from_mode() return the original channel pointer
if there is no attached channel with the requested mode.
But if so, that should be done differently.  I.e., Dan's
suggestion should be taken, and the callers should use the
passed-in channel if the call to get_channel_from_mode()
returns NULL.  (I hope that's clear.)

So anyway, I think this (and Dan's suggestion) should be
addressed, but your fix might not be correct.

Rui, can you please shed some light on the situation?

					-Alex

> 
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Signed-off-by: Mikhail Lobanov <m.lobanov@...alinux.ru>
> ---
>   drivers/staging/greybus/light.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 87d36948c610..929514350947 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -148,10 +148,15 @@ static int __gb_lights_flash_brightness_set(struct gb_channel *channel)
>   						GB_CHANNEL_MODE_TORCH);
>   
>   	/* For not flash we need to convert brightness to intensity */
> -	intensity = channel->intensity_uA.min +
> +
> +	if (channel) {
> +		intensity = channel->intensity_uA.min +
>   			(channel->intensity_uA.step * channel->led->brightness);
>   
> -	return __gb_lights_flash_intensity_set(channel, intensity);
> +		return __gb_lights_flash_intensity_set(channel, intensity);
> +	}
> +
> +	return 0;
>   }
>   #else
>   static struct gb_channel *get_channel_from_cdev(struct led_classdev *cdev)


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ