All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <nuno.sa@analog.com>
To: Arnav Kapoor <kapoorarnav43@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	David Lechner <dlechner@baylibre.com>,
	Andy Shevchenko <andy@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@intel.com>,
	Rodrigo Alencar <rodrigo.alencar@analog.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: core: fix signed integer overflow in iio_str_to_fixpoint()
Date: Fri, 23 Oct 2026 09:53:36 +0100	[thread overview]
Message-ID: <atsgYaApZPg_3Z1N@nsa> (raw)
In-Reply-To: <20261022140353.24213-1-kapoorarnav43@gmail.com>

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@gmail.com>
> ---

LGTM (maybe a Cc to stable next time but Jonathan typically takes care
of that),

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> 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@gmail.com/
> 
>  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
> 

      reply	other threads:[~2026-10-23  8:52 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 14:03 [PATCH v2] iio: core: fix signed integer overflow in iio_str_to_fixpoint() Arnav Kapoor
2026-10-23  8:53 ` Nuno Sá [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atsgYaApZPg_3Z1N@nsa \
    --to=nuno.sa@analog.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=kapoorarnav43@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.alencar@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.