linux-kernel - Re: [PATCH] iio: adc: palmas: fix off by one bugs

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 23 Apr 2023 14:11:24 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Dan Carpenter <dan.carpenter@...aro.org>
Cc:     Patrik Dahlström <risca@...akolonin.se>,
        Lars-Peter Clausen <lars@...afoo.de>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] iio: adc: palmas: fix off by one bugs

On Fri, 21 Apr 2023 13:41:56 +0300
Dan Carpenter <dan.carpenter@...aro.org> wrote:

> Valid values for "adc_chan" are zero to (PALMAS_ADC_CH_MAX - 1).
> Smatch detects some buffer overflows caused by this:
> drivers/iio/adc/palmas_gpadc.c:721 palmas_gpadc_read_event_value() error: buffer overflow 'adc->thresholds' 16 <= 16
> drivers/iio/adc/palmas_gpadc.c:758 palmas_gpadc_write_event_value() error: buffer overflow 'adc->thresholds' 16 <= 16
> 
> The effect of this bug in other functions is more complicated but
> obviously we should fix all of them.
> 
> Fixes: a99544c6c883 ("iio: adc: palmas: add support for iio threshold events")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>

Looks good to me.  Slight shuffle at the moment will delay me applying this.

I'll wait for Linus to pick up Greg's pull request then rebase my fixes branch
on top of that.  Otherwise I make a mess of linux-next ordering and things might
blow up.

In meantime, Patrik, please take a look.

Jonathan

> ---
> ---
>  drivers/iio/adc/palmas_gpadc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> index c1c439215aeb..7dfc9c927a23 100644
> --- a/drivers/iio/adc/palmas_gpadc.c
> +++ b/drivers/iio/adc/palmas_gpadc.c
> @@ -547,7 +547,7 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
>  	int adc_chan = chan->channel;
>  	int ret = 0;
>  
> -	if (adc_chan > PALMAS_ADC_CH_MAX)
> +	if (adc_chan >= PALMAS_ADC_CH_MAX)
>  		return -EINVAL;
>  
>  	mutex_lock(&adc->lock);
> @@ -595,7 +595,7 @@ static int palmas_gpadc_read_event_config(struct iio_dev *indio_dev,
>  	int adc_chan = chan->channel;
>  	int ret = 0;
>  
> -	if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
> +	if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
>  	mutex_lock(&adc->lock);
> @@ -684,7 +684,7 @@ static int palmas_gpadc_write_event_config(struct iio_dev *indio_dev,
>  	int adc_chan = chan->channel;
>  	int ret;
>  
> -	if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
> +	if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
>  	mutex_lock(&adc->lock);
> @@ -710,7 +710,7 @@ static int palmas_gpadc_read_event_value(struct iio_dev *indio_dev,
>  	int adc_chan = chan->channel;
>  	int ret;
>  
> -	if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
> +	if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
>  	mutex_lock(&adc->lock);
> @@ -744,7 +744,7 @@ static int palmas_gpadc_write_event_value(struct iio_dev *indio_dev,
>  	int old;
>  	int ret;
>  
> -	if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
> +	if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
>  	mutex_lock(&adc->lock);