[PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[Alexey Dobriyan]

This reverts commit 6e30111dbb4075e3c26c230417b32bc4a1c66831 .

1) this patch breaks scanf() which uses _parse_integer_limit()
   internally, and is supposed to get overflowed result
   (however incorrect it may be)

2) using check_mul_overflow() and check_add_overflow() as written
   doesn't add any overflow protection because it is under branch
   where such overflow can't happen if radix is kept <=16,

3) kernel-doc comment legitimize the function while exactly
   the opposite should be done. Not a fan.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/kstrtox.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index edc4eb7c1bca..97be2a39f537 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,30 +39,25 @@ const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
 	return s;
 }
 
-/**
- * _parse_integer_limit - Convert integer string representation to an integer
- * @s: Integer string representation
- * @base: Radix
- * @p: Where to store result
- * @max_chars: Maximum amount of characters to convert
- *
- * Convert non-negative integer string representation in explicitly given
- * radix to an integer. If overflow occurs, value at @p is set to ULLONG_MAX.
+/*
+ * Convert non-negative integer string representation in explicitly given radix
+ * to an integer. A maximum of max_chars characters will be converted.
  *
- * This function is the workhorse of other string conversion functions and it
- * is discouraged to use it explicitly. Consider kstrto*() family instead.
+ * Return number of characters consumed maybe or-ed with overflow bit.
+ * If overflow occurs, result integer (incorrect) is still returned.
  *
- * Return: Number of characters consumed, maybe ORed with overflow bit
+ * Don't you dare use this function.
  */
 noinline
 unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned long long *p,
 				  size_t max_chars)
 {
-	unsigned int rv, overflow = 0;
 	unsigned long long res;
+	unsigned int rv;
 
 	res = 0;
-	for (rv = 0; rv < max_chars; rv++, s++) {
+	rv = 0;
+	while (max_chars--) {
 		unsigned int c = *s;
 		unsigned int lc = _tolower(c);
 		unsigned int val;
@@ -81,17 +76,15 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
 		 * it in the max base we support (16)
 		 */
 		if (unlikely(res & (~0ull << 60))) {
-			if (check_mul_overflow(res, base, &res) ||
-			    check_add_overflow(res, val, &res)) {
-				res = ULLONG_MAX;
-				overflow = KSTRTOX_OVERFLOW;
-			}
-		} else {
-			res = res * base + val;
+			if (res > div_u64(ULLONG_MAX - val, base))
+				rv |= KSTRTOX_OVERFLOW;
 		}
+		res = res * base + val;
+		rv++;
+		s++;
 	}
 	*p = res;
-	return rv | overflow;
+	return rv;
 }
 
 noinline
-- 
2.54.0

Re: [PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[Dmitry Antipov]

On 7/2/26 9:06 PM, Alexey Dobriyan wrote:

> 1) this patch breaks scanf() which uses _parse_integer_limit() 
> internally, and is supposed to get overflowed result (however
> incorrect it may be)
> 
> 2) using check_mul_overflow() and check_add_overflow() as written 
> doesn't add any overflow protection because it is under branch where
> such overflow can't happen if radix is kept <=16,

It would be helpful to see an examples (preferably designed
as KUnit tests) to illustrate these breakages.

Dmitry

Re: [PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[Andy Shevchenko]

On Fri, Jul 03, 2026 at 08:55:03AM +0300, Dmitry Antipov wrote:
> On 7/2/26 9:06 PM, Alexey Dobriyan wrote:
> 
> > 1) this patch breaks scanf() which uses _parse_integer_limit()
> > internally, and is supposed to get overflowed result (however
> > incorrect it may be)
> > 
> > 2) using check_mul_overflow() and check_add_overflow() as written
> > doesn't add any overflow protection because it is under branch where
> > such overflow can't happen if radix is kept <=16,
> 
> It would be helpful to see an examples (preferably designed
> as KUnit tests) to illustrate these breakages.

+1. We need test cases, since there were none what is the real life example of
the breakage?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[Dmitry Antipov]

On 7/3/26 9:05 AM, Andy Shevchenko wrote:

> +1. We need test cases, since there were none what is the real life example of
> the breakage?

I've (qemu-)tried:

CONFIG_PRINTF_KUNIT_TEST=y
CONFIG_SCANF_KUNIT_TEST=y
CONFIG_CMDLINE_KUNIT_TEST=y

on x86_64, arm, and riscv - without seeing anything suspicious...

Dmitry

Re: [PATCH 1/1] Revert "lib: fix _parse_integer_limit() to handle overflow"

[Alexey Dobriyan]

On Fri, Jul 03, 2026 at 09:52:34AM +0300, Dmitry Antipov wrote:
> On 7/3/26 9:05 AM, Andy Shevchenko wrote:
> 
> > +1. We need test cases, since there were none what is the real life example of
> > the breakage?
> 
> I've (qemu-)tried:
> 
> CONFIG_PRINTF_KUNIT_TEST=y
> CONFIG_SCANF_KUNIT_TEST=y
> CONFIG_CMDLINE_KUNIT_TEST=y
> 
> on x86_64, arm, and riscv - without seeing anything suspicious...

This code was written in such a way to have common code between old
scanf/strtoul code (which parsed incorrectly) and new kstrt*() code.
scanf/strtoul instances were left alone to rot.

You can not do "res = ULLONG_MAX;"