All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes
@ 2021-04-05 11:44 Lars-Peter Clausen
  2021-04-05 14:55 ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Lars-Peter Clausen @ 2021-04-05 11:44 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jean-Baptiste Maneyrol, linux-iio, Lars-Peter Clausen

When setting the gyro or accelerometer scale the inv_mpu6050 driver ignores
the integer part of the value. As a result e.g. all of 0.13309, 1.13309,
12345.13309, ... are accepted as a valid gyro scale and 0.13309 is the
scale that gets set in all those cases.

Make sure to check that the integer part of the scale value is 0 and reject
it otherwise.

Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 453c51c79655..69ab94ab7297 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -731,12 +731,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
-static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val)
+static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val,
+					int val2)
 {
 	int result, i;
 
+	if (val != 0)
+		return -EINVAL;
+
 	for (i = 0; i < ARRAY_SIZE(gyro_scale_6050); ++i) {
-		if (gyro_scale_6050[i] == val) {
+		if (gyro_scale_6050[i] == val2) {
 			result = inv_mpu6050_set_gyro_fsr(st, i);
 			if (result)
 				return result;
@@ -767,13 +771,17 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
+static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val,
+					 int val2)
 {
 	int result, i;
 	u8 d;
 
+	if (val != 0)
+		return -EINVAL;
+
 	for (i = 0; i < ARRAY_SIZE(accel_scale); ++i) {
-		if (accel_scale[i] == val) {
+		if (accel_scale[i] == val2) {
 			d = (i << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
 			result = regmap_write(st->map, st->reg->accl_config, d);
 			if (result)
@@ -814,10 +822,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_ANGL_VEL:
-			result = inv_mpu6050_write_gyro_scale(st, val2);
+			result = inv_mpu6050_write_gyro_scale(st, val, val2);
 			break;
 		case IIO_ACCEL:
-			result = inv_mpu6050_write_accel_scale(st, val2);
+			result = inv_mpu6050_write_accel_scale(st, val, val2);
 			break;
 		default:
 			result = -EINVAL;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes
  2021-04-05 11:44 [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes Lars-Peter Clausen
@ 2021-04-05 14:55 ` Jonathan Cameron
  2021-04-06  7:51   ` Jean-Baptiste Maneyrol
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2021-04-05 14:55 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jean-Baptiste Maneyrol, linux-iio

On Mon,  5 Apr 2021 13:44:41 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> When setting the gyro or accelerometer scale the inv_mpu6050 driver ignores
> the integer part of the value. As a result e.g. all of 0.13309, 1.13309,
> 12345.13309, ... are accepted as a valid gyro scale and 0.13309 is the
> scale that gets set in all those cases.
> 
> Make sure to check that the integer part of the scale value is 0 and reject
> it otherwise.
> 
> Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Looks like this is in the 'obviously' correct category to me but
will leave it on list to give Jean-Baptiste a chance to look at it.

As ever, give me a poke if I seem to have lost it down the back of the
sofa in a few weeks time.

Thanks

Jonathan

> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 453c51c79655..69ab94ab7297 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -731,12 +731,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> -static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val)
> +static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val,
> +					int val2)
>  {
>  	int result, i;
>  
> +	if (val != 0)
> +		return -EINVAL;
> +
>  	for (i = 0; i < ARRAY_SIZE(gyro_scale_6050); ++i) {
> -		if (gyro_scale_6050[i] == val) {
> +		if (gyro_scale_6050[i] == val2) {
>  			result = inv_mpu6050_set_gyro_fsr(st, i);
>  			if (result)
>  				return result;
> @@ -767,13 +771,17 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
> +static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val,
> +					 int val2)
>  {
>  	int result, i;
>  	u8 d;
>  
> +	if (val != 0)
> +		return -EINVAL;
> +
>  	for (i = 0; i < ARRAY_SIZE(accel_scale); ++i) {
> -		if (accel_scale[i] == val) {
> +		if (accel_scale[i] == val2) {
>  			d = (i << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
>  			result = regmap_write(st->map, st->reg->accl_config, d);
>  			if (result)
> @@ -814,10 +822,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_ANGL_VEL:
> -			result = inv_mpu6050_write_gyro_scale(st, val2);
> +			result = inv_mpu6050_write_gyro_scale(st, val, val2);
>  			break;
>  		case IIO_ACCEL:
> -			result = inv_mpu6050_write_accel_scale(st, val2);
> +			result = inv_mpu6050_write_accel_scale(st, val, val2);
>  			break;
>  		default:
>  			result = -EINVAL;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes
  2021-04-05 14:55 ` Jonathan Cameron
@ 2021-04-06  7:51   ` Jean-Baptiste Maneyrol
  2021-04-06  8:37     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Jean-Baptiste Maneyrol @ 2021-04-06  7:51 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen; +Cc: linux-iio

Hi Jonathan, Lars-Peter,

thanks for letting me having a look. This is effectively as obvious as it looks.
Never think of testing that, thanks for the patch.

Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

Thanks,
JB

From: Jonathan Cameron <jic23@kernel.org>
Sent: Monday, April 5, 2021 16:55
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Subject: Re: [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes 
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

On Mon,  5 Apr 2021 13:44:41 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> When setting the gyro or accelerometer scale the inv_mpu6050 driver ignores
> the integer part of the value. As a result e.g. all of 0.13309, 1.13309,
> 12345.13309, ... are accepted as a valid gyro scale and 0.13309 is the
> scale that gets set in all those cases.
> 
> Make sure to check that the integer part of the scale value is 0 and reject
> it otherwise.
> 
> Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Looks like this is in the 'obviously' correct category to me but
will leave it on list to give Jean-Baptiste a chance to look at it.

As ever, give me a poke if I seem to have lost it down the back of the
sofa in a few weeks time.

Thanks

Jonathan

> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 453c51c79655..69ab94ab7297 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -731,12 +731,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>        }
>  }
>  
> -static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val)
> +static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val,
> +                                     int val2)
>  {
>        int result, i;
>  
> +     if (val != 0)
> +             return -EINVAL;
> +
>        for (i = 0; i < ARRAY_SIZE(gyro_scale_6050); ++i) {
> -             if (gyro_scale_6050[i] == val) {
> +             if (gyro_scale_6050[i] == val2) {
>                        result = inv_mpu6050_set_gyro_fsr(st, i);
>                        if (result)
>                                return result;
> @@ -767,13 +771,17 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
>        return -EINVAL;
>  }
>  
> -static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
> +static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val,
> +                                      int val2)
>  {
>        int result, i;
>        u8 d;
>  
> +     if (val != 0)
> +             return -EINVAL;
> +
>        for (i = 0; i < ARRAY_SIZE(accel_scale); ++i) {
> -             if (accel_scale[i] == val) {
> +             if (accel_scale[i] == val2) {
>                        d = (i << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
>                        result = regmap_write(st->map, st->reg->accl_config, d);
>                        if (result)
> @@ -814,10 +822,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
>        case IIO_CHAN_INFO_SCALE:
>                switch (chan->type) {
>                case IIO_ANGL_VEL:
> -                     result = inv_mpu6050_write_gyro_scale(st, val2);
> +                     result = inv_mpu6050_write_gyro_scale(st, val, val2);
>                        break;
>                case IIO_ACCEL:
> -                     result = inv_mpu6050_write_accel_scale(st, val2);
> +                     result = inv_mpu6050_write_accel_scale(st, val, val2);
>                        break;
>                default:
>                        result = -EINVAL;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes
  2021-04-06  7:51   ` Jean-Baptiste Maneyrol
@ 2021-04-06  8:37     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2021-04-06  8:37 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol; +Cc: Lars-Peter Clausen, linux-iio

On Tue, 6 Apr 2021 07:51:05 +0000
Jean-Baptiste Maneyrol <JManeyrol@invensense.com> wrote:

> Hi Jonathan, Lars-Peter,
> 
> thanks for letting me having a look. This is effectively as obvious as it looks.
> Never think of testing that, thanks for the patch.
> 
> Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

Applied to the togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> 
> Thanks,
> JB
> 
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Monday, April 5, 2021 16:55
> To: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
> Subject: Re: [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes 
>  
>  CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> On Mon,  5 Apr 2021 13:44:41 +0200
> Lars-Peter Clausen <lars@metafoo.de> wrote:
> 
> > When setting the gyro or accelerometer scale the inv_mpu6050 driver ignores
> > the integer part of the value. As a result e.g. all of 0.13309, 1.13309,
> > 12345.13309, ... are accepted as a valid gyro scale and 0.13309 is the
> > scale that gets set in all those cases.
> > 
> > Make sure to check that the integer part of the scale value is 0 and reject
> > it otherwise.
> > 
> > Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>  
> Looks like this is in the 'obviously' correct category to me but
> will leave it on list to give Jean-Baptiste a chance to look at it.
> 
> As ever, give me a poke if I seem to have lost it down the back of the
> sofa in a few weeks time.
> 
> Thanks
> 
> Jonathan
> 
> > ---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 453c51c79655..69ab94ab7297 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -731,12 +731,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> >        }
> >  }
> >  
> > -static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val)
> > +static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st, int val,
> > +                                     int val2)
> >  {
> >        int result, i;
> >  
> > +     if (val != 0)
> > +             return -EINVAL;
> > +
> >        for (i = 0; i < ARRAY_SIZE(gyro_scale_6050); ++i) {
> > -             if (gyro_scale_6050[i] == val) {
> > +             if (gyro_scale_6050[i] == val2) {
> >                        result = inv_mpu6050_set_gyro_fsr(st, i);
> >                        if (result)
> >                                return result;
> > @@ -767,13 +771,17 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
> >        return -EINVAL;
> >  }
> >  
> > -static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
> > +static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val,
> > +                                      int val2)
> >  {
> >        int result, i;
> >        u8 d;
> >  
> > +     if (val != 0)
> > +             return -EINVAL;
> > +
> >        for (i = 0; i < ARRAY_SIZE(accel_scale); ++i) {
> > -             if (accel_scale[i] == val) {
> > +             if (accel_scale[i] == val2) {
> >                        d = (i << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
> >                        result = regmap_write(st->map, st->reg->accl_config, d);
> >                        if (result)
> > @@ -814,10 +822,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> >        case IIO_CHAN_INFO_SCALE:
> >                switch (chan->type) {
> >                case IIO_ANGL_VEL:
> > -                     result = inv_mpu6050_write_gyro_scale(st, val2);
> > +                     result = inv_mpu6050_write_gyro_scale(st, val, val2);
> >                        break;
> >                case IIO_ACCEL:
> > -                     result = inv_mpu6050_write_accel_scale(st, val2);
> > +                     result = inv_mpu6050_write_accel_scale(st, val, val2);
> >                        break;
> >                default:
> >                        result = -EINVAL;  


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-06  8:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-05 11:44 [PATCH] iio: inv_mpu6050: Fully validate gyro and accel scale writes Lars-Peter Clausen
2021-04-05 14:55 ` Jonathan Cameron
2021-04-06  7:51   ` Jean-Baptiste Maneyrol
2021-04-06  8:37     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.