Re: [PATCH] iio: inkern: Avoid risky abs() usage in iio_multiply_value()

From: Romain Gantois

Date: Tue Mar 31 2026 - 08:18:35 EST


Hello Andy,

On Tuesday, 31 March 2026 12:08:26 CEST Andy Shevchenko wrote:
> On Tue, Mar 31, 2026 at 12:29:22PM +0300, Andy Shevchenko wrote:
> > On Tue, Mar 31, 2026 at 10:49:59AM +0200, Romain Gantois wrote:
> ...
>
> > > - *result = multiplier * abs(val);
> > > - *result += div_s64(multiplier * abs(val2), denominator);
> > > + *result = multiplier * abs((s64)val);
> > > + *result += div_s64(multiplier * abs((s64)val2), denominator);
> >
> > Right, but here we get val and val2 from either static values from the
> > driver (when it is SCALE channel), or when channel has PROCESSED support.
> > In the latter one it might theoretically be possible to go till the
> > INT_MIN, but practically I don't know how, except for the broken driver
> > code in the first place. With that being said, I think it's better to
> > validate somewhere the multipliers (when it's SCALE or PROCESSED
> > channel). I also noted that for the _PROCESSED some drivers keep a
> > garbage in val2. That probably needs to be addressed as well (exempli
> > gratia: bmi270_read_raw() does that).
>
> Actually the data in the val and val2 should be aligned with the returned
> type, hence the potential bugs might only come from the untested drivers.
> Which means that this patch doesn't improve the situation.

I'm a bit confused: when you say "the returned type" what returning function
are you referring to? Also, doesn't the patch still fix the bug for potentially
untested drivers which use PROCESSED?

Thanks,

--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: This is a digitally signed message part.