iio: chemical: scd30: Prevent potential divide-by-zero errors

[PATCH v2] iio: chemical: scd30: Prevent potential divide-by-zero errors

Posted by Maxwell Doose 1 month ago

In scd30_read_raw, the current value of tmp in the
IIO_CHAN_INFO_SAMP_FREQ case is unchecked. Add checking to see if the
value we got was 0 to prevent a divide-by-zero error.

A similar case has also been found in scd30_write_raw(), also in the
IIO_CHAN_INFO_SAMP_FREQ case, where the value of val2 was unchecked.
Add checking for that variable as well and return -EINVAL if it's 0.

Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 v2:
 - Switch to dev_err_ratelimited() per sashiko.
 - Fix another potential divide-by-zero error per sashiko (see commit
   message).

 drivers/iio/chemical/scd30_core.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
index be8c055be184..3851c9334c8b 100644
--- a/drivers/iio/chemical/scd30_core.c
+++ b/drivers/iio/chemical/scd30_core.c
@@ -237,6 +237,16 @@ static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
 		if (ret)
 			return ret;
 
+		/*
+		 * Value of 0 is unexpected but possible if hardware is failing
+		 * or noise on data bus
+		 */
+		if (!tmp) {
+			dev_err_ratelimited(&indio_dev->dev,
+					    "Invalid measurement interval 0 received\n");
+			return -EIO;
+		}
+
 		*val = 0;
 		*val2 = 1000000000 / tmp;
 		return IIO_VAL_INT_PLUS_NANO;
@@ -261,7 +271,7 @@ static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
 	guard(mutex)(&state->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val)
+		if (val || !val2)
 			return -EINVAL;
 
 		val = 1000000000 / val2;
-- 
2.54.0

Re: [PATCH v2] iio: chemical: scd30: Prevent potential divide-by-zero errors

Posted by Jonathan Cameron 1 month ago

On Sun, 10 May 2026 15:21:56 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> In scd30_read_raw, the current value of tmp in the
> IIO_CHAN_INFO_SAMP_FREQ case is unchecked. Add checking to see if the
> value we got was 0 to prevent a divide-by-zero error.
> 
> A similar case has also been found in scd30_write_raw(), also in the
> IIO_CHAN_INFO_SAMP_FREQ case, where the value of val2 was unchecked.
> Add checking for that variable as well and return -EINVAL if it's 0.
> 
> Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
To me this one is a hardening change not a bug fix (unless we have
reports of this hardware failure on a real product - not a devboard
with dodgy wiring ;)

So no fixes tag needed.

Also there is already a fix for the write_raw one in my fixes-togreg
branch. Antoniu got there a few weeks back.

Jonathan



> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  v2:
>  - Switch to dev_err_ratelimited() per sashiko.
>  - Fix another potential divide-by-zero error per sashiko (see commit
>    message).
> 
>  drivers/iio/chemical/scd30_core.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> index be8c055be184..3851c9334c8b 100644
> --- a/drivers/iio/chemical/scd30_core.c
> +++ b/drivers/iio/chemical/scd30_core.c
> @@ -237,6 +237,16 @@ static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
>  		if (ret)
>  			return ret;
>  
> +		/*
> +		 * Value of 0 is unexpected but possible if hardware is failing
> +		 * or noise on data bus
> +		 */
> +		if (!tmp) {
> +			dev_err_ratelimited(&indio_dev->dev,
> +					    "Invalid measurement interval 0 received\n");
> +			return -EIO;
> +		}
> +
>  		*val = 0;
>  		*val2 = 1000000000 / tmp;
>  		return IIO_VAL_INT_PLUS_NANO;
> @@ -261,7 +271,7 @@ static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
>  	guard(mutex)(&state->lock);
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		if (val)
> +		if (val || !val2)
>  			return -EINVAL;
>  
>  		val = 1000000000 / val2;

Re: [PATCH v2] iio: chemical: scd30: Prevent potential divide-by-zero errors

Posted by Jonathan Cameron 1 month ago

On Mon, 11 May 2026 13:05:55 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 10 May 2026 15:21:56 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
> 
> > In scd30_read_raw, the current value of tmp in the
> > IIO_CHAN_INFO_SAMP_FREQ case is unchecked. Add checking to see if the
> > value we got was 0 to prevent a divide-by-zero error.
> > 
> > A similar case has also been found in scd30_write_raw(), also in the
> > IIO_CHAN_INFO_SAMP_FREQ case, where the value of val2 was unchecked.
> > Add checking for that variable as well and return -EINVAL if it's 0.
> > 
> > Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")  
> To me this one is a hardening change not a bug fix (unless we have
> reports of this hardware failure on a real product - not a devboard
> with dodgy wiring ;)
> 
> So no fixes tag needed.
> 
> Also there is already a fix for the write_raw one in my fixes-togreg
> branch. Antoniu got there a few weeks back.

By which I meant no fixes tag needed when it's just the read_raw case
where it isn't a userspace controlled value

J
> 
> Jonathan
> 
> 
> 
> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> > ---
> >  v2:
> >  - Switch to dev_err_ratelimited() per sashiko.
> >  - Fix another potential divide-by-zero error per sashiko (see commit
> >    message).
> > 
> >  drivers/iio/chemical/scd30_core.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> > index be8c055be184..3851c9334c8b 100644
> > --- a/drivers/iio/chemical/scd30_core.c
> > +++ b/drivers/iio/chemical/scd30_core.c
> > @@ -237,6 +237,16 @@ static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
> >  		if (ret)
> >  			return ret;
> >  
> > +		/*
> > +		 * Value of 0 is unexpected but possible if hardware is failing
> > +		 * or noise on data bus
> > +		 */
> > +		if (!tmp) {
> > +			dev_err_ratelimited(&indio_dev->dev,
> > +					    "Invalid measurement interval 0 received\n");
> > +			return -EIO;
> > +		}
> > +
> >  		*val = 0;
> >  		*val2 = 1000000000 / tmp;
> >  		return IIO_VAL_INT_PLUS_NANO;
> > @@ -261,7 +271,7 @@ static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
> >  	guard(mutex)(&state->lock);
> >  	switch (mask) {
> >  	case IIO_CHAN_INFO_SAMP_FREQ:
> > -		if (val)
> > +		if (val || !val2)
> >  			return -EINVAL;
> >  
> >  		val = 1000000000 / val2;  
>

Re: [PATCH v2] iio: chemical: scd30: Prevent potential divide-by-zero errors

Posted by Andy Shevchenko 1 month ago

On Sun, May 10, 2026 at 03:21:56PM -0500, Maxwell Doose wrote:
> In scd30_read_raw, the current value of tmp in the

We refer to the functions as func().

> IIO_CHAN_INFO_SAMP_FREQ case is unchecked. Add checking to see if the
> value we got was 0 to prevent a divide-by-zero error.
> 
> A similar case has also been found in scd30_write_raw(), also in the
> IIO_CHAN_INFO_SAMP_FREQ case, where the value of val2 was unchecked.
> Add checking for that variable as well and return -EINVAL if it's 0.

...

> +		/*
> +		 * Value of 0 is unexpected but possible if hardware is failing
> +		 * or noise on data bus

Respect English punctuation. Here the period at the end is missing.

> +		 */

> +			dev_err_ratelimited(&indio_dev->dev,
> +					    "Invalid measurement interval 0 received\n");

I believe here is missing colon.

					    "Invalid measurement: interval 0 received\n");

> +			return -EIO;
> +		}

-- 
With Best Regards,
Andy Shevchenko