Re: [PATCH v2] iio: core: fix signed integer overflow in iio_str_to_fixpoint()

[Jonathan Cameron]

On Fri, 23 Oct 2026 09:53:36 +0100
Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:

> On Thu, Oct 22, 2026 at 07:33:51PM +0530, Arnav Kapoor wrote:
> > __iio_str_to_fixpoint() accumulates the integer part of the parsed number
> > into an int. Feeding it a string with more digits than an int can hold
> > overflows that variable, which is undefined behaviour.
> > 
> > The parser is reachable from userspace via the writable IIO channel
> > attributes, for example the "sampling_frequency" or "in_voltage_scale"
> > ones, and via the exported iio_str_to_fixpoint() helper used by a
> > number of IIO drivers. A user writing a long enough string therefore
> > triggers a signed integer overflow.
> > 
> > Use check_mul_overflow() and check_add_overflow() to accumulate the
> > integer part and return -ERANGE when it does not fit into an int. The
> > sign is applied while accumulating, so INT_MIN can still be parsed.
> > 
> > Fixes: 6807d7211327 ("iio: Factor out fixed point number parsing into its own function")
> > Signed-off-by: Arnav Kapoor <kapoorarnav43@xxxxxxxxx>
> > ---  
> 
> LGTM (maybe a Cc to stable next time but Jonathan typically takes care
> of that),
> 
> Reviewed-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

In in two minds about this.  It is preventing unintuitive behaviour but
we shouldn't have any cases that crash or anything like that.

Given the code in question is likely to be replaced shortly anyway
by Rodrigo's rework applying this for backporting purposes makes me nervous
even though it looks fine. I also don't want to delay Rodrigo's work
by having this go in via a different branch.

So perhaps the worst possible compromise.  Applied to the testing branch
of iio.git (which I'll rebase on rc1 shortly) and marked for stable.

Rodrigo, you are going to need to rebase your series on top of this.

Thanks

Jonathan



> 
> > Changes in v2:
> > - Use check_mul_overflow()/check_add_overflow() instead of open-coded
> >   s64 accumulation and limits (Andy)
> > - Apply the sign while accumulating so INT_MIN needs no special case
> > - Drop the unrelated parenthesis change and the explicit casts (Andy)
> > - Add Fixes tag (Andy)
> > - Fix the truncated subject line
> > - No KUnit tests for the current parser, as it is replaced by the
> >   E notation series (Rodrigo)
> > 
> > The fractional part is left as is: with the fract_mult values used in
> > tree (at most 10^8) it cannot exceed 999999999.
> > 
> > Tested with a userspace harness built with -fsanitize=undefined on
> > inputs including "2147483647", "2147483648", "-2147483648",
> > "-2147483649", "99999999999999999999", "-0.5" and "12.345\n".
> > All inputs that fit into an int parse exactly as before. Compile-tested
> > in tree with W=1. Applies to both togreg and fixes-togreg.
> > 
> > v1: https://lore.kernel.org/all/20261021225306.623361-1-kapoorarnav43@xxxxxxxxx/
> > 
> >  drivers/iio/industrialio-core.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index d73f2541375f..f3bf17c13cea 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/math64.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/overflow.h>
> >  #include <linux/poll.h>
> >  #include <linux/property.h>
> >  #include <linux/sched.h>
> > @@ -1027,7 +1028,12 @@ static int __iio_str_to_fixpoint(const char *str, int fract_mult,
> >  	while (*str) {
> >  		if ('0' <= *str && *str <= '9') {
> >  			if (integer_part) {
> > -				i = i * 10 + *str - '0';
> > +				int digit = *str - '0';
> > +
> > +				/* Accumulate with the sign applied so INT_MIN fits */
> > +				if (check_mul_overflow(i, 10, &i) ||
> > +				    check_add_overflow(i, negative ? -digit : digit, &i))
> > +					return -ERANGE;
> >  			} else {
> >  				f += fract_mult * (*str - '0');
> >  				fract_mult /= 10;
> > @@ -1052,12 +1058,8 @@ static int __iio_str_to_fixpoint(const char *str, int fract_mult,
> >  		str++;
> >  	}
> >  
> > -	if (negative) {
> > -		if (i)
> > -			i = -i;
> > -		else
> > -			f = -f;
> > -	}
> > +	if (negative && !i)
> > +		f = -f;
> >  
> >  	*integer = i;
> >  	*fract = f;
> > 
> > base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
> > -- 
> > 2.53.0
> >