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: <byitgzjli5gsq5v66topve7ip3inkk2udwhuihjdp6bknnkmos@tv226l7tek7s>
Date: Mon, 26 Jan 2026 12:42:53 +0000
From: Rodrigo Alencar <455.rodrigo.alencar@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>, 
	rodrigo.alencar@...log.com
Cc: 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 26/01/26 01:49PM, Andy Shevchenko wrote:
> On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> 
> > Add iio_str_to_fixpoint64() function that leverages simple_strtoull()
> > to parse numbers from a string.
> > A helper function __iio_str_to_fixpoint64() replaces
> > __iio_str_to_fixpoint() implementation, extending its usage for
> > 64-bit fixed-point parsing.

...

> > +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?
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

on overflow of u64, the function would return -EOVERFLOW

> that will do all necessary checks and returns -EINVAL, -ERANGE, et cetera.
> In the below we would need check for the error codes respectively.
> 
> > +	if (precision && *end == '.') {
> > +		str = end + 1;
> > +		f = simple_strtoull(str, &end, 10);
> > +		digit_count = end - str;
> > +		if (!digit_count || digit_count > 20)
> > +			return -EINVAL;
> > +
> > +		if (digit_count > precision) {
> > +			digit_count -= precision;
> > +			f = div64_u64(f, int_pow(10, digit_count));
> > +		} else {
> > +			digit_count = precision - digit_count;
> > +			f *= int_pow(10, digit_count);
> > +		}
> > +	} else if (!digit_count) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (scale_db) {
> 
> > +		/* Ignore the dB suffix */
> > +		if (!strncmp(end, " dB", sizeof(" dB") - 1))
> > +			end += sizeof(" dB") - 1;
> > +		else if (!strncmp(end, "dB", sizeof("dB") - 1))
> > +			end += sizeof("dB") - 1;
> 
> Now we have strends()

strends() would not account for the acceptable '\n' before the end.
I don't think we would need to test for " dB", " dB\n", "dB" and "dB\n"

...

> > +	if (integer64 < INT_MIN || integer64 > INT_MAX ||
> > +	    fract64 < INT_MIN || fract64 > INT_MAX)
> > +		return -ERANGE;
> 
> This needs to account the sign, right?
> It's fine to be UINT_MAX I believe. But I haven't checked the current
> implementation.

Yes, UINT_MAX should be valid, will add test case and adjust. Thanks.

Kind regards,

Rodrigo Alencar