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

[Romain Gantois <romain.gantois@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

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 #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]