* [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; 4+ 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] 4+ 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
2026-10-22 8:24 ` Arnav Kapoor
0 siblings, 1 reply; 4+ 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] 4+ messages in thread* Re: [PATCH] iio: core: fix signed integer overflow in
2026-10-22 7:08 ` Andy Shevchenko
@ 2026-10-22 8:24 ` Arnav Kapoor
2026-10-22 10:16 ` Rodrigo Alencar
0 siblings, 1 reply; 4+ messages in thread
From: Arnav Kapoor @ 2026-10-22 8:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: jic23, rodrigo.alencar, dlechner, nuno.sa, andy, linux-iio, linux-kernel
(Resending with the list Cc'd, sorry for the duplicate, Andy.)
On Thu, Oct 22, 2026 at 10:08:10AM +0300, Andy Shevchenko wrote:
> This needs a Fixes tag, otherwise it probably won't needed as Rodrigo working
> towards tons of improvements in this area.
Will add:
Fixes: 6807d7211327 ("iio: Factor out fixed point number parsing into its own function")
> OTOH, Rodrigo's test cases might be good as part of this fix as well.
As far as I can see, Rodrigo's tests in the E notation RFC exercise
kstrtodec64() in lib/test-kstrtox.c, so they won't cover the current
parser. I'd rather keep the fix small for backporting and send KUnit
cases for iio_str_to_fixpoint() as a separate patch, if that's OK.
> Wow, what's wrong with INT_MIN?
Nothing, it was just clumsy. In v2 the sign is applied while
accumulating, so INT_MIN falls out naturally and the limit goes away.
> What's the point in changing this line?
> Why do you need explicit castings?
None, both dropped.
> Do you know about APIs in overflow.h? I think you wanted to use something
> from there.
Yes, v2 uses check_mul_overflow() and check_add_overflow() and keeps
the variables as int, so the s64 and the declaration reordering are
gone too.
I'll send v2 shortly.
Thanks,
Arnav
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iio: core: fix signed integer overflow in
2026-10-22 8:24 ` Arnav Kapoor
@ 2026-10-22 10:16 ` Rodrigo Alencar
0 siblings, 0 replies; 4+ messages in thread
From: Rodrigo Alencar @ 2026-10-22 10:16 UTC (permalink / raw)
To: Arnav Kapoor, Andy Shevchenko
Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On 22/10/26 13:54, Arnav Kapoor wrote:
> (Resending with the list Cc'd, sorry for the duplicate, Andy.)
>
> On Thu, Oct 22, 2026 at 10:08:10AM +0300, Andy Shevchenko wrote:
> > This needs a Fixes tag, otherwise it probably won't needed as Rodrigo working
> > towards tons of improvements in this area.
>
> Will add:
>
> Fixes: 6807d7211327 ("iio: Factor out fixed point number parsing into its own function")
>
> > OTOH, Rodrigo's test cases might be good as part of this fix as well.
>
> As far as I can see, Rodrigo's tests in the E notation RFC exercise
> kstrtodec64() in lib/test-kstrtox.c, so they won't cover the current
> parser. I'd rather keep the fix small for backporting and send KUnit
> cases for iio_str_to_fixpoint() as a separate patch, if that's OK.
The current parser is being dropped in the E notation series, so that
overflow concerns are handled differently. iio_str_to_fixpoint() is being
tested there indirectly with tests on iio_write_channel_info(), maybe not
is thorough as it should, but it tries to focus on different format types
and coverage. Dedicated parsing tests with different inputs are handled
in lib/test-kstrtox.c. With that, not sure you need a KUnit test for the
current parser. Only small fix meant for backporting would make sense to me.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-22 10:16 UTC | newest]
Thread overview: 4+ 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
2026-10-22 8:24 ` Arnav Kapoor
2026-10-22 10:16 ` Rodrigo Alencar
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®