* [PATCH] iio: adc: palmas: fix off by one bugs
@ 2023-04-21 10:41 Dan Carpenter
2023-04-23 13:11 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2023-04-21 10:41 UTC (permalink / raw)
To: Patrik Dahlström
Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, linux-kernel,
kernel-janitors
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@linaro.org>
---
---
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);
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: palmas: fix off by one bugs
2023-04-21 10:41 [PATCH] iio: adc: palmas: fix off by one bugs Dan Carpenter
@ 2023-04-23 13:11 ` Jonathan Cameron
2023-04-29 20:25 ` Patrik Dahlström
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2023-04-23 13:11 UTC (permalink / raw)
To: Dan Carpenter
Cc: Patrik Dahlström, Lars-Peter Clausen, linux-iio,
linux-kernel, kernel-janitors
On Fri, 21 Apr 2023 13:41:56 +0300
Dan Carpenter <dan.carpenter@linaro.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@linaro.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);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: palmas: fix off by one bugs
2023-04-23 13:11 ` Jonathan Cameron
@ 2023-04-29 20:25 ` Patrik Dahlström
2023-05-01 15:06 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Patrik Dahlström @ 2023-04-29 20:25 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Dan Carpenter, Lars-Peter Clausen, linux-iio, linux-kernel,
kernel-janitors
On Sun, Apr 23, 2023 at 02:11:24PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Apr 2023 13:41:56 +0300
> Dan Carpenter <dan.carpenter@linaro.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@linaro.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.
Sorry for the long delay.
The changes look good to me. I've checked all other uses of adc_chan in the
code and they all seem to be guarded by the checks below.
Best regards
Patrik
>
> 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);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: palmas: fix off by one bugs
2023-04-29 20:25 ` Patrik Dahlström
@ 2023-05-01 15:06 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2023-05-01 15:06 UTC (permalink / raw)
To: Patrik Dahlström
Cc: Dan Carpenter, Lars-Peter Clausen, linux-iio, linux-kernel,
kernel-janitors
On Sat, 29 Apr 2023 22:25:38 +0200
Patrik Dahlström <risca@dalakolonin.se> wrote:
> On Sun, Apr 23, 2023 at 02:11:24PM +0100, Jonathan Cameron wrote:
> > On Fri, 21 Apr 2023 13:41:56 +0300
> > Dan Carpenter <dan.carpenter@linaro.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@linaro.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.
>
> Sorry for the long delay.
>
> The changes look good to me. I've checked all other uses of adc_chan in the
> code and they all seem to be guarded by the checks below.
>
> Best regards
> Patrik
>
My tree has now advanced appropriately.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> >
> > 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);
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-01 14:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-21 10:41 [PATCH] iio: adc: palmas: fix off by one bugs Dan Carpenter
2023-04-23 13:11 ` Jonathan Cameron
2023-04-29 20:25 ` Patrik Dahlström
2023-05-01 15:06 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox