iio: humidity: hts221: fix division by zero in calibration data parsing

[PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Yang Zi 1 month ago

hts221_parse_temp_caldata() and hts221_parse_rh_caldata() compute the
sensor slope as ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0). If the
device reports cal_x1 == cal_x0 the division causes a divide-by-zero
error.

Return -EINVAL when cal_x1 == cal_x0 in both functions before performing
the division.

Signed-off-by: Yang Zi <2959243019@qq.com>
---
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index bfeb0a60d3af..7d1f04951d8b 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -288,6 +288,9 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
         return err;
     cal_x1 = le16_to_cpu(val);
 
+    if (cal_x1 == cal_x0)
+        return -EINVAL;
+
     slope = &hw->sensors[HTS221_SENSOR_T].slope;
     b_gen = &hw->sensors[HTS221_SENSOR_T].b_gen;
 
@@ -327,6 +330,9 @@ static int hts221_parse_rh_caldata(struct hts221_hw *hw)
         return err;
     cal_x1 = le16_to_cpu(val);
 
+    if (cal_x1 == cal_x0)
+        return -EINVAL;
+
     slope = &hw->sensors[HTS221_SENSOR_H].slope;
     b_gen = &hw->sensors[HTS221_SENSOR_H].b_gen;

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Andy Shevchenko 1 month ago

On Tue, Aug 25, 2026 at 05:21:10PM +0800, Yang Zi wrote:
> hts221_parse_temp_caldata() and hts221_parse_rh_caldata() compute the
> sensor slope as ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0). If the
> device reports cal_x1 == cal_x0 the division causes a divide-by-zero
> error.
> 
> Return -EINVAL when cal_x1 == cal_x0 in both functions before performing
> the division.

Do you have HW to test? Any fault injection perhaps?
The problem with div-by-0 is that it's implementation defined, meaning
that it's how compiler decides. Now, the question is, what will compiler
do in the current code (no patch applied)?

...

>          return err;

>      cal_x1 = le16_to_cpu(val);

>  

I would remove this blank line as it's coupled with the above.

> +    if (cal_x1 == cal_x0)
> +        return -EINVAL;
> +

>      slope = &hw->sensors[HTS221_SENSOR_T].slope;
>      b_gen = &hw->sensors[HTS221_SENSOR_T].b_gen;

Ditto for the rest.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Jonathan Cameron 4 weeks ago

On Wed, 26 Aug 2026 17:09:49 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Aug 25, 2026 at 05:21:10PM +0800, Yang Zi wrote:
> > hts221_parse_temp_caldata() and hts221_parse_rh_caldata() compute the
> > sensor slope as ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0). If the
> > device reports cal_x1 == cal_x0 the division causes a divide-by-zero
> > error.
> > 
> > Return -EINVAL when cal_x1 == cal_x0 in both functions before performing
> > the division.  
> 
> Do you have HW to test? Any fault injection perhaps?
> The problem with div-by-0 is that it's implementation defined, meaning
> that it's how compiler decides. Now, the question is, what will compiler
> do in the current code (no patch applied)?
> 
Whilst all this is true, the code shouldn't be relying on any particular
behavior.  So do we need to know?

Reality is the hardware is almost certainly never returning values where this
is true.

Jonathan

> ...
> 
> >          return err;  
> 
> >      cal_x1 = le16_to_cpu(val);  
> 
> >    
> 
> I would remove this blank line as it's coupled with the above.
> 
> > +    if (cal_x1 == cal_x0)
> > +        return -EINVAL;
> > +  
> 
> >      slope = &hw->sensors[HTS221_SENSOR_T].slope;
> >      b_gen = &hw->sensors[HTS221_SENSOR_T].b_gen;  
> 
> Ditto for the rest.
> 
>

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Andy Shevchenko 3 weeks, 6 days ago

On Mon, Aug 31, 2026 at 01:00:21AM +0100, Jonathan Cameron wrote:
> On Wed, 26 Aug 2026 17:09:49 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Tue, Aug 25, 2026 at 05:21:10PM +0800, Yang Zi wrote:

...

> > Do you have HW to test? Any fault injection perhaps?
> > The problem with div-by-0 is that it's implementation defined, meaning
> > that it's how compiler decides. Now, the question is, what will compiler
> > do in the current code (no patch applied)?
> > 
> Whilst all this is true, the code shouldn't be relying on any particular
> behavior.  So do we need to know?

That's my point. If code relies on that, it should be spelled clearly in
the commit message. Then we will deliberately break that (nasty and wrong)
behaviour.

> Reality is the hardware is almost certainly never returning values where this
> is true.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Jonathan Cameron 3 weeks, 6 days ago

On Mon, 31 Aug 2026 10:05:06 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Aug 31, 2026 at 01:00:21AM +0100, Jonathan Cameron wrote:
> > On Wed, 26 Aug 2026 17:09:49 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Tue, Aug 25, 2026 at 05:21:10PM +0800, Yang Zi wrote:  
> 
> ...
> 
> > > Do you have HW to test? Any fault injection perhaps?
> > > The problem with div-by-0 is that it's implementation defined, meaning
> > > that it's how compiler decides. Now, the question is, what will compiler
> > > do in the current code (no patch applied)?
> > >   
> > Whilst all this is true, the code shouldn't be relying on any particular
> > behavior.  So do we need to know?  
> 
> That's my point. If code relies on that, it should be spelled clearly in
> the commit message. Then we will deliberately break that (nasty and wrong)
> behaviour.
> 

Ah. Got you. So a statement that the condition (probably) never occurs in practice
should do the job here by side stepping that question.

> > Reality is the hardware is almost certainly never returning values where this
> > is true.  
>

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Andy Shevchenko 3 weeks, 5 days ago

On Tue, Sep 01, 2026 at 02:35:44AM +0100, Jonathan Cameron wrote:
> On Mon, 31 Aug 2026 10:05:06 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Mon, Aug 31, 2026 at 01:00:21AM +0100, Jonathan Cameron wrote:
> > > On Wed, 26 Aug 2026 17:09:49 +0300
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > > On Tue, Aug 25, 2026 at 05:21:10PM +0800, Yang Zi wrote:  

...

> > > > Do you have HW to test? Any fault injection perhaps?
> > > > The problem with div-by-0 is that it's implementation defined, meaning
> > > > that it's how compiler decides. Now, the question is, what will compiler
> > > > do in the current code (no patch applied)?
> > > >   
> > > Whilst all this is true, the code shouldn't be relying on any particular
> > > behavior.  So do we need to know?  
> > 
> > That's my point. If code relies on that, it should be spelled clearly in
> > the commit message. Then we will deliberately break that (nasty and wrong)
> > behaviour.
> 
> Ah. Got you. So a statement that the condition (probably) never occurs in practice
> should do the job here by side stepping that question.

Yep!

> > > Reality is the hardware is almost certainly never returning values where this
> > > is true.  

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Joshua Crofts 1 month ago

On Tue, 25 Aug 2026 17:21:10 +0800
Yang Zi <2959243019@qq.com> wrote:

> hts221_parse_temp_caldata() and hts221_parse_rh_caldata() compute the
> sensor slope as ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0). If the
> device reports cal_x1 == cal_x0 the division causes a divide-by-zero
> error.
> 
> Return -EINVAL when cal_x1 == cal_x0 in both functions before performing
> the division.
> 
> Signed-off-by: Yang Zi <2959243019@qq.com>
> ---
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index bfeb0a60d3af..7d1f04951d8b 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -288,6 +288,9 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
>          return err;
>      cal_x1 = le16_to_cpu(val);
>  
> +    if (cal_x1 == cal_x0)
> +        return -EINVAL;
> +
>      slope = &hw->sensors[HTS221_SENSOR_T].slope;
>      b_gen = &hw->sensors[HTS221_SENSOR_T].b_gen;
>  
> @@ -327,6 +330,9 @@ static int hts221_parse_rh_caldata(struct hts221_hw *hw)
>          return err;
>      cal_x1 = le16_to_cpu(val);
>  
> +    if (cal_x1 == cal_x0)
> +        return -EINVAL;
> +
>      slope = &hw->sensors[HTS221_SENSOR_H].slope;
>      b_gen = &hw->sensors[HTS221_SENSOR_H].b_gen;
>  
> 
> 

This definitely needs a Fixes: tag, otherwise LGTM.

Feel free to carry my review tag.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

PS, a bit of process info: I noticed Sashiko (sashiko.dev) failed to apply your
patches. To prevent this from happening in the future, please use the `--base` flag
when running `git format-patch` to specify which commit it should be applied onto.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH] iio: humidity: hts221: fix division by zero in calibration data parsing

Posted by Jonathan Cameron 4 weeks ago

On Tue, 25 Aug 2026 11:39:58 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Tue, 25 Aug 2026 17:21:10 +0800
> Yang Zi <2959243019@qq.com> wrote:
> 
> > hts221_parse_temp_caldata() and hts221_parse_rh_caldata() compute the
> > sensor slope as ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0). If the
> > device reports cal_x1 == cal_x0 the division causes a divide-by-zero
> > error.
> > 
> > Return -EINVAL when cal_x1 == cal_x0 in both functions before performing
> > the division.
> > 
> > Signed-off-by: Yang Zi <2959243019@qq.com>
> > ---
> > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > index bfeb0a60d3af..7d1f04951d8b 100644
> > --- a/drivers/iio/humidity/hts221_core.c
> > +++ b/drivers/iio/humidity/hts221_core.c
> > @@ -288,6 +288,9 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
> >          return err;
> >      cal_x1 = le16_to_cpu(val);
> >  
> > +    if (cal_x1 == cal_x0)
> > +        return -EINVAL;
> > +
> >      slope = &hw->sensors[HTS221_SENSOR_T].slope;
> >      b_gen = &hw->sensors[HTS221_SENSOR_T].b_gen;
> >  
> > @@ -327,6 +330,9 @@ static int hts221_parse_rh_caldata(struct hts221_hw *hw)
> >          return err;
> >      cal_x1 = le16_to_cpu(val);
> >  
> > +    if (cal_x1 == cal_x0)
> > +        return -EINVAL;
> > +
> >      slope = &hw->sensors[HTS221_SENSOR_H].slope;
> >      b_gen = &hw->sensors[HTS221_SENSOR_H].b_gen;
> >  
> > 
> >   
> 
> This definitely needs a Fixes: tag, otherwise LGTM.
Looks like hardening to me rather than a fix. Given those algs come
off the datasheet and the div zero case makes no sense, I'd not expect
the hardware to ever return a combination that triggers this.

With that said, the cost in catching it is low.

I was going to pick this up, but seems patch is corrupted.
Looks like we have tabs replaced with white space but I haven't checked closely.

Please fix that up and send a v2.

thanks,

Jonathan


> 
> Feel free to carry my review tag.
> 
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> PS, a bit of process info: I noticed Sashiko (sashiko.dev) failed to apply your
> patches. To prevent this from happening in the future, please use the `--base` flag
> when running `git format-patch` to specify which commit it should be applied onto.
>