linux-kernel - Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXd_0uNVm8c_Fhwd@smile.fi.intel.com>
Date: Mon, 26 Jan 2026 16:53:06 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Rodrigo Alencar <455.rodrigo.alencar@...il.com>
Cc: rodrigo.alencar@...log.com, linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org, Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Andy Shevchenko <andy@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit
 parts

On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 03:35PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:

...

> > > > > +static int __iio_str_to_fixpoint64(const char *str, u64 fract_mult,
> > > > > +				   s64 *integer, s64 *fract, bool scale_db)
> > > > > +{
> > > > > +	u64 i = 0, f = 0;
> > > > > +	char *end;
> > > > > +	int digit_count, precision = ffs(fract_mult);
> > > > > +	bool negative = false;
> > > > > +
> > > > > +	if (str[0] == '-') {
> > > > > +		negative = true;
> > > > > +		str++;
> > > > > +	} else if (str[0] == '+') {
> > > > > +		str++;
> > > > > +	}
> > > > > +
> > > > > +	i = simple_strtoull(str, &end, 10);
> > > > > +	digit_count = end - str;
> > > > > +	if (digit_count > 20)
> > > > > +		return -EINVAL;
> > > > 
> > > > Not really. If we are talking about decimal (only) cases we need to also count
> > > > leading 0:s.
> > > > 
> > > > 0000000000000000000000000000000025 is still 25, no overflow.
> > > > 
> > > > That's why I recommend to have a helper, maybe for now locally here, like
> > > > 
> > > > int safe_strtoull(..., unsigned long long *res)
> > > > {
> > > > 	...
> > > > }
> > > 
> > > Are you suggesting to not use simple_strtoull then?
> > 
> > Nope, I suggest to do an additional step before checking for the range.
> 
> You mean, conditionally skip leading 0's when parsing the integer part?
> e.g.
> 
> /*function entry and arg check */
> while(*str == '\0')
> 	str++;
> /* then call simple_strtoull() */

Not skipping, but counting them.

> simple_strtoull() is not overflow-safe,

Yes, I know. That's why all these additional checks are required,

> as it does not use
> check_mul_overflow() or check_add_overflow(), only checking the
> amount of digits is not enough.

Why? Can you elaborate how checking amount of digits is different to
check_mul_overflow()?

> Previous implementation of fixpoint parsing didn't care about that.

Do we have test cases for the current implementation?

> > > Understood, leading zeros can be ignored only when parsing the integer 
> > > part. Also, would be nice to have truncation of the fractional part
> > > while doing the parsing. How about:
> > > 
> > > static int iio_safe_strtoull(const char *str, const char **end,
> > > 			     size_t max_chars, u64 *res)
> > 
> > > - max_chars = 0: ignores leading 0's and process all digits
> > > - max_chars > 0: process only initial max_chars digits and ignores the rest
> > 
> > I'm not sure why we would need that. It should parse the whole line until the
> > first invalid character or overflow.
> 
> "process all digits" and "ignores the rest" would be for digits only, so it
> would stop until the first invalid character is found. I suppose proper
> overflow check is implemented with check_mul_overflow() and check_add_overflow(),

I don't see the need. Amount of digits defines the order of the number (in
power-of-ten).

> while iterating over the characters and accumulating the value.

The problem that you can refer to is the corner case when the first
(most significant digit(s)) are already give an overflow while being
inside the allowed length. But it also can be checked.

...

The benefit of simple_strto*() over kstrto*() that they do not require
a temporary buffer and work over constant data (always).

If you see a way how to refactor lib/kstrtox.c and lib/vsprintf.c to have
an implementation there directly that may operate over constant buffers,
I will be glad to help with it. That would be good for existing cases,
such as Intel QAT driver, and any newcomers. I actually don't know why
the heck kstrto*() were made against non-constant buffers. Perhaps to
avoid this 'end' parameter...

-- 
With Best Regards,
Andy Shevchenko