iio: accel: bma400: integer underflow setting accel scale
diff mbox series

Message ID 20200115174531.p623ukjibn6kg6zz@kili.mountain
State New
Headers show
Series
  • iio: accel: bma400: integer underflow setting accel scale
Related show

Commit Message

Dan Carpenter Jan. 15, 2020, 5:45 p.m. UTC
We put an upper bound on "val2" but we also need to prevent negative
values.

Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/iio/accel/bma400_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dan Robertson Jan. 15, 2020, 5:43 p.m. UTC | #1
Thanks for taking a look at the code and your feedback on the driver!

On Wed, Jan 15, 2020 at 08:45:31PM +0300, Dan Carpenter wrote:
> We put an upper bound on "val2" but we also need to prevent negative
> values.

"val" is not used past the invalid value check. We only use "val" to make sure
that it is in fact 0. AFAIK there is no "upper bound" on "val", it should be
zero or we return -EINVAL. Am I missing something?

Cheers,

 - Dan
Dan Robertson Jan. 15, 2020, 5:58 p.m. UTC | #2
On Wed, Jan 15, 2020 at 09:09:01PM +0300, Dan Carpenter wrote:
> On Wed, Jan 15, 2020 at 05:43:24PM +0000, Dan Robertson wrote:
> > Thanks for taking a look at the code and your feedback on the driver!
> > 
> > On Wed, Jan 15, 2020 at 08:45:31PM +0300, Dan Carpenter wrote:
> > > We put an upper bound on "val2" but we also need to prevent negative
> > > values.
> > 
> > "val" is not used past the invalid value check. We only use "val" to make sure
> > that it is in fact 0. AFAIK there is no "upper bound" on "val", it should be
> > zero or we return -EINVAL. Am I missing something?
> 
> This patch affects "val2" not "val".  ;)

Ah! Right, my bad :/ Good catch!

Cheers,

 - Dan
Dan Carpenter Jan. 15, 2020, 6:09 p.m. UTC | #3
On Wed, Jan 15, 2020 at 05:43:24PM +0000, Dan Robertson wrote:
> Thanks for taking a look at the code and your feedback on the driver!
> 
> On Wed, Jan 15, 2020 at 08:45:31PM +0300, Dan Carpenter wrote:
> > We put an upper bound on "val2" but we also need to prevent negative
> > values.
> 
> "val" is not used past the invalid value check. We only use "val" to make sure
> that it is in fact 0. AFAIK there is no "upper bound" on "val", it should be
> zero or we return -EINVAL. Am I missing something?

This patch affects "val2" not "val".  ;)

regards,
dan carpenter
Dan Robertson Jan. 15, 2020, 6:17 p.m. UTC | #4
On Wed, Jan 15, 2020 at 08:45:31PM +0300, Dan Carpenter wrote:
> We put an upper bound on "val2" but we also need to prevent negative
> values.
> 
> Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/iio/accel/bma400_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index ab4a158b35af..ffc7b146bbfc 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -752,7 +752,7 @@ static int bma400_write_raw(struct iio_dev *indio_dev,
>  		mutex_unlock(&data->mutex);
>  		return ret;
>  	case IIO_CHAN_INFO_SCALE:
> -		if (val != 0 || val2 > BMA400_SCALE_MAX)
> +		if (val != 0 || val2 < 0 || val2 > BMA400_SCALE_MAX)

AFAIK if val2 is less than BMA400_SCALE_MIN we should return -EINVAL.

Cheers,

 -Dan
Dan Carpenter Jan. 16, 2020, 6:24 a.m. UTC | #5
On Wed, Jan 15, 2020 at 06:17:17PM +0000, Dan Robertson wrote:
> On Wed, Jan 15, 2020 at 08:45:31PM +0300, Dan Carpenter wrote:
> > We put an upper bound on "val2" but we also need to prevent negative
> > values.
> > 
> > Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >  drivers/iio/accel/bma400_core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index ab4a158b35af..ffc7b146bbfc 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
> > @@ -752,7 +752,7 @@ static int bma400_write_raw(struct iio_dev *indio_dev,
> >  		mutex_unlock(&data->mutex);
> >  		return ret;
> >  	case IIO_CHAN_INFO_SCALE:
> > -		if (val != 0 || val2 > BMA400_SCALE_MAX)
> > +		if (val != 0 || val2 < 0 || val2 > BMA400_SCALE_MAX)
> 
> AFAIK if val2 is less than BMA400_SCALE_MIN we should return -EINVAL.
> 

Ah.  Thanks.  Let me resend.

regards,
dan carpenter

Patch
diff mbox series

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index ab4a158b35af..ffc7b146bbfc 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -752,7 +752,7 @@  static int bma400_write_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&data->mutex);
 		return ret;
 	case IIO_CHAN_INFO_SCALE:
-		if (val != 0 || val2 > BMA400_SCALE_MAX)
+		if (val != 0 || val2 < 0 || val2 > BMA400_SCALE_MAX)
 			return -EINVAL;
 
 		mutex_lock(&data->mutex);