Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] iio: accel: bma400: integer underflow setting accel scale
  2020-01-15 17:45 [PATCH] iio: accel: bma400: integer underflow setting accel scale Dan Carpenter
@ 2020-01-15 17:43 ` Dan Robertson
  2020-01-15 18:09   ` Dan Carpenter
  2020-01-15 18:17 ` Dan Robertson
  1 sibling, 1 reply; 8+ messages in thread
From: Dan Robertson @ 2020-01-15 17:43 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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


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

* [PATCH] iio: accel: bma400: integer underflow setting accel scale
@ 2020-01-15 17:45 Dan Carpenter
  2020-01-15 17:43 ` Dan Robertson
  2020-01-15 18:17 ` Dan Robertson
  0 siblings, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-01-15 17:45 UTC (permalink / raw)
  To: Dan Robertson
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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)
 			return -EINVAL;
 
 		mutex_lock(&data->mutex);
-- 
2.11.0


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

* Re: [PATCH] iio: accel: bma400: integer underflow setting accel scale
  2020-01-15 18:09   ` Dan Carpenter
@ 2020-01-15 17:58     ` Dan Robertson
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Robertson @ 2020-01-15 17:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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


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

* Re: [PATCH] iio: accel: bma400: integer underflow setting accel scale
  2020-01-15 17:43 ` Dan Robertson
@ 2020-01-15 18:09   ` Dan Carpenter
  2020-01-15 17:58     ` Dan Robertson
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2020-01-15 18:09 UTC (permalink / raw)
  To: Dan Robertson
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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


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

* Re: [PATCH] iio: accel: bma400: integer underflow setting accel scale
  2020-01-15 17:45 [PATCH] iio: accel: bma400: integer underflow setting accel scale Dan Carpenter
  2020-01-15 17:43 ` Dan Robertson
@ 2020-01-15 18:17 ` Dan Robertson
  2020-01-16  6:24   ` Dan Carpenter
  2020-01-16 10:08   ` [PATCH v2] iio: accel: bma400: prevent setting accel scale too low Dan Carpenter
  1 sibling, 2 replies; 8+ messages in thread
From: Dan Robertson @ 2020-01-15 18:17 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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


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

* Re: [PATCH] iio: accel: bma400: integer underflow setting accel scale
  2020-01-15 18:17 ` Dan Robertson
@ 2020-01-16  6:24   ` Dan Carpenter
  2020-01-16 10:08   ` [PATCH v2] iio: accel: bma400: prevent setting accel scale too low Dan Carpenter
  1 sibling, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-01-16  6:24 UTC (permalink / raw)
  To: Dan Robertson
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko,
	linux-iio, kernel-janitors

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


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

* [PATCH v2] iio: accel: bma400: prevent setting accel scale too low
  2020-01-15 18:17 ` Dan Robertson
  2020-01-16  6:24   ` Dan Carpenter
@ 2020-01-16 10:08   ` Dan Carpenter
  2020-01-18 13:34     ` Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2020-01-16 10:08 UTC (permalink / raw)
  To: Dan Robertson
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Andy Shevchenko, Linus Walleij,
	linux-iio, kernel-janitors

This puts an upper bound on "val2" but it also needs to have a lower
bound (BMA400_SCALE_MIN).

Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: the first version just capped it at zero but it should be
    BMA400_SCALE_MIN (38357).

 drivers/iio/accel/bma400_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index ab4a158b35af..cc77f89c048b 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -752,7 +752,8 @@ 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 < BMA400_SCALE_MIN || val2 > BMA400_SCALE_MAX)
 			return -EINVAL;
 
 		mutex_lock(&data->mutex);
-- 
2.11.0


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

* Re: [PATCH v2] iio: accel: bma400: prevent setting accel scale too low
  2020-01-16 10:08   ` [PATCH v2] iio: accel: bma400: prevent setting accel scale too low Dan Carpenter
@ 2020-01-18 13:34     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-01-18 13:34 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Dan Robertson, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Andy Shevchenko,
	Linus Walleij, linux-iio, kernel-janitors

On Thu, 16 Jan 2020 13:08:29 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> This puts an upper bound on "val2" but it also needs to have a lower
> bound (BMA400_SCALE_MIN).
> 
> Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied to the togreg branch of iio.git and pushed out as testing.
Hopefully I'll sneak in a pull request to get this lined up for
the merge window.

Thanks,

Jonathan

> ---
> v2: the first version just capped it at zero but it should be
>     BMA400_SCALE_MIN (38357).
> 
>  drivers/iio/accel/bma400_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index ab4a158b35af..cc77f89c048b 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -752,7 +752,8 @@ 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 < BMA400_SCALE_MIN || val2 > BMA400_SCALE_MAX)
>  			return -EINVAL;
>  
>  		mutex_lock(&data->mutex);


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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15 17:45 [PATCH] iio: accel: bma400: integer underflow setting accel scale Dan Carpenter
2020-01-15 17:43 ` Dan Robertson
2020-01-15 18:09   ` Dan Carpenter
2020-01-15 17:58     ` Dan Robertson
2020-01-15 18:17 ` Dan Robertson
2020-01-16  6:24   ` Dan Carpenter
2020-01-16 10:08   ` [PATCH v2] iio: accel: bma400: prevent setting accel scale too low Dan Carpenter
2020-01-18 13:34     ` Jonathan Cameron

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git