| Message ID | 20200115174531.p623ukjibn6kg6zz@kili.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: accel: bma400: integer underflow setting accel scale | expand |
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
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
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
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
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
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);
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(-)