From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>,
Dmitry Antipov <dmantipov@yandex.ru>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Kees Cook <kees@kernel.org>, Petr Mladek <pmladek@suse.com>
Cc: rodrigo.alencar@analog.com, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>
Subject: Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts
Date: Tue, 27 Jan 2026 09:43:55 +0200 [thread overview]
Message-ID: <aXhsu2DvG-5PLOEU@smile.fi.intel.com> (raw)
In-Reply-To: <anhtxco52jz2ktmxlittsjuvfqybwrwgy76bjhni3j5dzx2rh2@hpzf3sfwe7hf>
Remove DT related people and ML and add others based on the links I posted
below.
On Mon, Jan 26, 2026 at 04:55:13PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 06:07PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> > > > On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
...
> > > > > Why? Can you elaborate how checking amount of digits is different to
> > > > > check_mul_overflow()?
> > > >
> > > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> > > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> > > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
> > > >
> > > > to catch those cases, we need to check for the overflow, everytime we read a
> > > > character and accumulate:
> > > >
> > > > u64 acc;
> > > >
> > > > while(isdigit(*str))
> > > > if (check_mul_overflow(acc, 10, &acc) ||
> > > > check_add_overflow(acc, *str - '0', &acc))
> > > > return -EOVERFLOW;
> > > >
> > > > *res = acc;
> > > >
> > > > acc can get weird results if not checked.
> > >
> > > Thinking about it again, that check could be done only in the last step
> > > (20th for u64)
> >
> > Does kstrto*() also perform only last check? I think they do for each
> > iteration.
>
> It does the following:
>
> ...
> if (unlikely(res & (~0ull << 60))) {
> if (res > div_u64(ULLONG_MAX - val, base))
> rv |= KSTRTOX_OVERFLOW;
> }
> ...
>
> so overflow is checked when either one of the 4 MSbits are set.
>
> for now, I am thinking of something like:
>
> static ssize_t iio_safe_strtou64(const char *str, const char **endp,
> size_t max_chars, u64 *result)
> {
> u64 digit, acc = 0;
> size_t idx = 0;
>
> while (isdigit(*str) && idx < max_chars) {
> digit = *str - '0';
> if (unlikely(idx > 19)) {
> if (check_mul_overflow(acc, 10, &acc) ||
> check_add_overflow(acc, digit, &acc))
> return -EOVERFLOW;
> } else {
> acc = acc * 10 + digit;
> }
> str++;
> idx++;
> }
>
> *endp = str;
> *result = acc;
> return idx;
> }
>
> which would help the truncation when parsing the fractional part
> with max_chars, avoiding a div64_u64() to adjust precision:
>
> ...
> digit_count = iio_safe_strtou64(str, &end, SIZE_MAX, &i);
> if (digit_count < 0)
> return digit_count;
>
> if (precision && *end == '.') {
> str = end + 1;
> digit_count = iio_safe_strtou64(str, &end, precision, &f);
> if (digit_count < 0)
> return digit_count;
>
> if (digit_count < precision) /* scale up */
> f *= int_pow(10, precision - digit_count);
>
> while (isdigit(*end)) /* truncate */
> end++;
> }
> ...
>
> but I understand you would not like this approach, because it does not use
> simple_strtoull() or kstrtoull(). Problem is simple_strtoull() is not
> overflow-safe and kstrtoull() does not allow to track a pointer to end
> of the string.
>
> Given that the current implementation of iio_str_to_fixpoint() is not using
> simple_strtoull() I am not seeing an issue with this approach.
I believe this is the goal, id est to get rid of the code duplication.
The idea is not exactly in _using_ simple_strto*() or kstrto*(), but
deriving the common parts that can be reused here. For simplicity, we
may leave iio_str_to_fixpoint() alone for now (as you mentioned it
doesn't share currently the code, so can be addressed later on)
and try to provide a treewide available safe_strto*().
Browsing through lore.kernel.org I found these
(in backward chronological order):
https://lore.kernel.org/linux-hardening/20260126162059.357467-1-dmantipov@yandex.ru/
https://lore.kernel.org/lkml/d6c3b369-9777-9986-f41f-3f3a4f85d64c@rasmusvillemoes.dk/
https://lore.kernel.org/lkml/CA+55aFyC7N4S65UVrp0Hcefb5AsMPqGn_bco6tFL+JZ0m3nh=A@mail.gmail.com/
which suggests that the problems are known and there are attempts to address them.
Perhaps we should consider what Rasmus started 6 years ago...
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2026-01-27 7:44 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 15:53 [PATCH v5 0/8] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 1/8] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 16:14 ` Rodrigo Alencar
2026-01-23 16:29 ` Rodrigo Alencar
2026-01-26 11:49 ` Andy Shevchenko
2026-01-26 12:42 ` Rodrigo Alencar
2026-01-26 13:35 ` Andy Shevchenko
2026-01-26 14:26 ` Rodrigo Alencar
2026-01-26 14:53 ` Andy Shevchenko
2026-01-26 15:20 ` Rodrigo Alencar
2026-01-26 15:30 ` Rodrigo Alencar
2026-01-26 16:07 ` Andy Shevchenko
2026-01-26 16:55 ` Rodrigo Alencar
2026-01-27 7:43 ` Andy Shevchenko [this message]
2026-01-27 10:17 ` Rodrigo Alencar
2026-01-31 18:03 ` Jonathan Cameron
2026-01-26 16:05 ` Andy Shevchenko
2026-01-26 14:56 ` Rodrigo Alencar
2026-01-26 15:06 ` Andy Shevchenko
2026-01-23 15:53 ` [PATCH v5 3/8] iio: test: add kunit test for fixed-point parsing Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 4/8] iio: frequency: adf41513: driver implementation Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 5/8] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 6/8] iio: frequency: adf41513: features on frequency change Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 7/8] docs: iio: add documentation for adf41513 driver Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 8/8] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar
2026-01-23 15:53 ` Rodrigo Alencar via B4 Relay
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=aXhsu2DvG-5PLOEU@smile.fi.intel.com \
--to=andriy.shevchenko@intel.com \
--cc=455.rodrigo.alencar@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=dmantipov@yandex.ru \
--cc=jic23@kernel.org \
--cc=kees@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=pmladek@suse.com \
--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.