mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] iio: core: fix signed integer overflow in
@ 2026-10-21 22:52 Arnav Kapoor
  2026-10-22  7:08 ` Andy Shevchenko
  0 siblings, 1 reply; 2+ messages in thread
From: Arnav Kapoor @ 2026-10-21 22:52 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko
  Cc: Arnav Kapoor, linux-iio, linux-kernel

__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 same happens
when the parsed value is INT_MIN and the sign is applied afterwards.

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.

Accumulate the integer and the fractional part in a s64 and reject
values that do not fit into an int with -ERANGE. The limit for the
integer part depends on the sign as the sign is applied only after the
whole string has been parsed.

Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
---
 drivers/iio/industrialio-core.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d73f2541375f..40f4375163c3 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1008,7 +1008,8 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
 static int __iio_str_to_fixpoint(const char *str, int fract_mult,
 				 int *integer, int *fract, bool scale_db)
 {
-	int i = 0, f = 0;
+	s64 i = 0, f = 0;
+	s64 limit;
 	bool integer_part = true, negative = false;
 
 	if (fract_mult == 0) {
@@ -1024,12 +1025,21 @@ static int __iio_str_to_fixpoint(const char *str, int fract_mult,
 		str++;
 	}
 
+	/*
+	 * The integer part is accumulated as a magnitude and the sign is only
+	 * applied once the whole string has been parsed. Hence the limit
+	 * depends on the sign.
+	 */
+	limit = negative ? (s64)INT_MAX + 1 : INT_MAX;
+
 	while (*str) {
 		if ('0' <= *str && *str <= '9') {
 			if (integer_part) {
-				i = i * 10 + *str - '0';
+				i = i * 10 + (*str - '0');
+				if (i > limit)
+					return -ERANGE;
 			} else {
-				f += fract_mult * (*str - '0');
+				f += (s64)fract_mult * (*str - '0');
 				fract_mult /= 10;
 			}
 		} else if (*str == '\n') {
@@ -1059,8 +1069,11 @@ static int __iio_str_to_fixpoint(const char *str, int fract_mult,
 			f = -f;
 	}
 
-	*integer = i;
-	*fract = f;
+	if (f < INT_MIN || f > INT_MAX)
+		return -ERANGE;
+
+	*integer = (int)i;
+	*fract = (int)f;
 
 	return 0;
 }


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] iio: core: fix signed integer overflow in
  2026-10-21 22:52 [PATCH] iio: core: fix signed integer overflow in Arnav Kapoor
@ 2026-10-22  7:08 ` Andy Shevchenko
  0 siblings, 0 replies; 2+ messages in thread
From: Andy Shevchenko @ 2026-10-22  7:08 UTC (permalink / raw)
  To: Arnav Kapoor
  Cc: Jonathan Cameron, Rodrigo Alencar, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

+Cc: Rodrigo

On Thu, Oct 22, 2026 at 04:22:48AM +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 same happens
> when the parsed value is INT_MIN and the sign is applied afterwards.
> 
> 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.
> 
> Accumulate the integer and the fractional part in a s64 and reject
> values that do not fit into an int with -ERANGE. The limit for the
> integer part depends on the sign as the sign is applied only after the
> whole string has been parsed.

This needs a Fixes tag, otherwise it probably won't needed as Rodrigo working
towards tons of improvements in this area.

OTOH, Rodrigo's test cases might be good as part of this fix as well.

...

> -	int i = 0, f = 0;
> +	s64 i = 0, f = 0;
> +	s64 limit;
>  	bool integer_part = true, negative = false;

While at it, make it reversed xmas tree order.

...

> +	/*
> +	 * The integer part is accumulated as a magnitude and the sign is only
> +	 * applied once the whole string has been parsed. Hence the limit
> +	 * depends on the sign.
> +	 */
> +	limit = negative ? (s64)INT_MAX + 1 : INT_MAX;

Wow, what's wrong with INT_MIN?

>  	while (*str) {
>  		if ('0' <= *str && *str <= '9') {
>  			if (integer_part) {
> -				i = i * 10 + *str - '0';
> +				i = i * 10 + (*str - '0');

What's the point in changing this line?

> +				if (i > limit)
> +					return -ERANGE;
>  			} else {
> -				f += fract_mult * (*str - '0');
> +				f += (s64)fract_mult * (*str - '0');
>  				fract_mult /= 10;
>  			}
>  		} else if (*str == '\n') {

>  	}

...

> +	*integer = (int)i;
> +	*fract = (int)f;

Why do you need explicit castings?

...

Do you know about APIs in overflow.h? I think you wanted to use something
from there.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-22  7:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-21 22:52 [PATCH] iio: core: fix signed integer overflow in Arnav Kapoor
2026-10-22  7:08 ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®