mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges
@ 2026-10-05 21:39 Armaan Sandhu
  2026-10-07 22:00 ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Armaan Sandhu @ 2026-10-05 21:39 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Shevchenko, linux-kernel, Armaan Sandhu, stable

get_range() stops writing once the array is full, but get_options()
still advances its index by the whole range. So "1-100" parsed into
four ints reports 99 numbers, and a range like "0-2147483647" wraps
the index negative and writes outside the array.

Stop once a range fills the array, bail out in validation mode before
the count overflows, and saturate the range length in get_range().
Add KUnit cases; without the fix "0-2147483647" panics the test kernel.

Only root can supply this input, so this is a robustness fix.

Fixes: 22f2e2801799 ("[PATCH] get_options to allow a hypenated range for isolcpus")
Cc: stable@vger.kernel.org
Signed-off-by: Armaan Sandhu <armaan.sandhu0504@gmail.com>
---
 lib/cmdline.c             | 13 ++++++++++++-
 lib/tests/cmdline_kunit.c | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 16cce6621cec..40b98d943a9a 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -11,6 +11,7 @@
 
 #include <linux/export.h>
 #include <linux/kernel.h>
+#include <linux/overflow.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
 
@@ -26,7 +27,9 @@ static int get_range(char **str, int *pint, int n)
 
 	(*str)++;
 	upper_range = simple_strtol((*str), NULL, 0);
-	inc_counter = upper_range - *pint;
+	/* Keep the sign of the result when the difference doesn't fit */
+	if (check_sub_overflow(upper_range, *pint, &inc_counter))
+		inc_counter = upper_range < *pint ? -1 : INT_MAX;
 	for (x = *pint; n && x < upper_range; x++, n--)
 		*pint++ = x;
 	return inc_counter;
@@ -122,6 +125,14 @@ char *get_options(const char *str, int nints, int *ints)
 			range_nums = get_range((char **)&str, pint, n);
 			if (range_nums < 0)
 				break;
+			/* The range didn't fit, so the array is full */
+			if (!validate && range_nums > n) {
+				i = nints;
+				break;
+			}
+			/* Leave room for the upper number of the range */
+			if (range_nums >= INT_MAX - i)
+				break;
 			/*
 			 * Decrement the result by one to leave out the
 			 * last number in the range.  The next iteration
diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
index 3f61ff8d3178..584fcb2c0e44 100644
--- a/lib/tests/cmdline_kunit.c
+++ b/lib/tests/cmdline_kunit.c
@@ -140,6 +140,43 @@ static void cmdline_test_range(struct kunit *test)
 	} while (++i < ARRAY_SIZE(cmdline_test_range_strings));
 }
 
+static const struct {
+	const char *in;
+	int parsed[4];
+	int validated;
+} cmdline_test_range_overflow_cases[] = {
+	{ "1-100",         { 3, 1, 2, 3, },    100,        },
+	{ "1,5-100,7",     { 3, 1, 5, 6, },    98,         },
+	{ "1-2147483646",  { 3, 1, 2, 3, },    2147483646, },
+	{ "0-2147483647",  { 3, 0, 1, 2, },    0,          },
+	{ "-5-2147483647", { 3, -5, -4, -3, }, 0,          },
+	{ "2147483647--5", { 0, 2147483647, }, 0,          },
+};
+
+static void cmdline_test_range_overflow(struct kunit *test)
+{
+	unsigned int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(cmdline_test_range_overflow_cases); i++) {
+		const char *in = cmdline_test_range_overflow_cases[i].in;
+		const int *e = cmdline_test_range_overflow_cases[i].parsed;
+		/* Two guard elements past the array handed to get_options() */
+		int r[ARRAY_SIZE(cmdline_test_range_overflow_cases[0].parsed) + 2];
+		int n;
+
+		memset(r, 0, sizeof(r));
+		get_options(in, ARRAY_SIZE(r) - 2, r);
+		for (j = 0; j < ARRAY_SIZE(r) - 2; j++)
+			KUNIT_EXPECT_EQ_MSG(test, r[j], e[j], "Pattern: %s at %u", in, j);
+		for (; j < ARRAY_SIZE(r); j++)
+			KUNIT_EXPECT_EQ_MSG(test, r[j], 0, "Pattern: %s out of bound at %u", in, j);
+
+		get_options(in, 0, &n);
+		KUNIT_EXPECT_EQ_MSG(test, n, cmdline_test_range_overflow_cases[i].validated,
+				    "Pattern: %s (validated)", in);
+	}
+}
+
 static void cmdline_test_next_arg_quoted_value(struct kunit *test)
 {
 	char in[] = "foo=\"bar baz\" qux=1";
@@ -258,6 +295,7 @@ static struct kunit_case cmdline_test_cases[] = {
 	KUNIT_CASE(cmdline_test_lead_int),
 	KUNIT_CASE(cmdline_test_tail_int),
 	KUNIT_CASE(cmdline_test_range),
+	KUNIT_CASE(cmdline_test_range_overflow),
 	KUNIT_CASE(cmdline_test_next_arg_quoted_value),
 	KUNIT_CASE(cmdline_test_next_arg_bare_quote_regression),
 	KUNIT_CASE(cmdline_test_next_arg_mixed_tokens),
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges
  2026-10-05 21:39 [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges Armaan Sandhu
@ 2026-10-07 22:00 ` Andrew Morton
       [not found]   ` <CAGD6qbSfTmHq7Y_jM7-QS26at=w6OQwfOZjCU8n4PnnFmqgDNg@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2026-10-07 22:00 UTC (permalink / raw)
  To: Armaan Sandhu; +Cc: Andy Shevchenko, linux-kernel, stable, lzhan011

On Mon,  5 Oct 2026 17:39:45 -0400 Armaan Sandhu <armaan.sandhu0504@gmail.com> wrote:

> get_range() stops writing once the array is full, but get_options()
> still advances its index by the whole range. So "1-100" parsed into
> four ints reports 99 numbers, and a range like "0-2147483647" wraps
> the index negative and writes outside the array.
> 
> Stop once a range fills the array, bail out in validation mode before
> the count overflows, and saturate the range length in get_range().
> Add KUnit cases; without the fix "0-2147483647" panics the test kernel.
> 
> Only root can supply this input, so this is a robustness fix.
> 
> Fixes: 22f2e2801799 ("[PATCH] get_options to allow a hypenated range for isolcpus")
> Cc: stable@vger.kernel.org
> Signed-off-by: Armaan Sandhu <armaan.sandhu0504@gmail.com>

I've received two fixes for the same 20 year old bug with an hour
(https://lore.kernel.org/20261005202412.3460441-1-lzsx618@gmail.com). 
How did that happen?

>  lib/cmdline.c             | 13 ++++++++++++-
>  lib/tests/cmdline_kunit.c | 38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)

The kunit changes appear to be identical.  Which fix is best?

> diff --git a/lib/cmdline.c b/lib/cmdline.c
> index 16cce6621cec..40b98d943a9a 100644
> --- a/lib/cmdline.c
> +++ b/lib/cmdline.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/export.h>
>  #include <linux/kernel.h>
> +#include <linux/overflow.h>
>  #include <linux/string.h>
>  #include <linux/ctype.h>
>  
> @@ -26,7 +27,9 @@ static int get_range(char **str, int *pint, int n)
>  
>  	(*str)++;
>  	upper_range = simple_strtol((*str), NULL, 0);
> -	inc_counter = upper_range - *pint;
> +	/* Keep the sign of the result when the difference doesn't fit */
> +	if (check_sub_overflow(upper_range, *pint, &inc_counter))
> +		inc_counter = upper_range < *pint ? -1 : INT_MAX;
>  	for (x = *pint; n && x < upper_range; x++, n--)
>  		*pint++ = x;
>  	return inc_counter;
> @@ -122,6 +125,14 @@ char *get_options(const char *str, int nints, int *ints)
>  			range_nums = get_range((char **)&str, pint, n);
>  			if (range_nums < 0)
>  				break;
> +			/* The range didn't fit, so the array is full */
> +			if (!validate && range_nums > n) {
> +				i = nints;
> +				break;
> +			}
> +			/* Leave room for the upper number of the range */
> +			if (range_nums >= INT_MAX - i)
> +				break;
>  			/*
>  			 * Decrement the result by one to leave out the
>  			 * last number in the range.  The next iteration
> diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
> index 3f61ff8d3178..584fcb2c0e44 100644
> --- a/lib/tests/cmdline_kunit.c
> +++ b/lib/tests/cmdline_kunit.c
> @@ -140,6 +140,43 @@ static void cmdline_test_range(struct kunit *test)
>  	} while (++i < ARRAY_SIZE(cmdline_test_range_strings));
>  }
>  
> +static const struct {
> +	const char *in;
> +	int parsed[4];
> +	int validated;
> +} cmdline_test_range_overflow_cases[] = {
> +	{ "1-100",         { 3, 1, 2, 3, },    100,        },
> +	{ "1,5-100,7",     { 3, 1, 5, 6, },    98,         },
> +	{ "1-2147483646",  { 3, 1, 2, 3, },    2147483646, },
> +	{ "0-2147483647",  { 3, 0, 1, 2, },    0,          },
> +	{ "-5-2147483647", { 3, -5, -4, -3, }, 0,          },
> +	{ "2147483647--5", { 0, 2147483647, }, 0,          },
> +};
> +
> +static void cmdline_test_range_overflow(struct kunit *test)
> +{
> +	unsigned int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(cmdline_test_range_overflow_cases); i++) {
> +		const char *in = cmdline_test_range_overflow_cases[i].in;
> +		const int *e = cmdline_test_range_overflow_cases[i].parsed;
> +		/* Two guard elements past the array handed to get_options() */
> +		int r[ARRAY_SIZE(cmdline_test_range_overflow_cases[0].parsed) + 2];
> +		int n;
> +
> +		memset(r, 0, sizeof(r));
> +		get_options(in, ARRAY_SIZE(r) - 2, r);
> +		for (j = 0; j < ARRAY_SIZE(r) - 2; j++)
> +			KUNIT_EXPECT_EQ_MSG(test, r[j], e[j], "Pattern: %s at %u", in, j);
> +		for (; j < ARRAY_SIZE(r); j++)
> +			KUNIT_EXPECT_EQ_MSG(test, r[j], 0, "Pattern: %s out of bound at %u", in, j);
> +
> +		get_options(in, 0, &n);
> +		KUNIT_EXPECT_EQ_MSG(test, n, cmdline_test_range_overflow_cases[i].validated,
> +				    "Pattern: %s (validated)", in);
> +	}
> +}
> +
>  static void cmdline_test_next_arg_quoted_value(struct kunit *test)
>  {
>  	char in[] = "foo=\"bar baz\" qux=1";
> @@ -258,6 +295,7 @@ static struct kunit_case cmdline_test_cases[] = {
>  	KUNIT_CASE(cmdline_test_lead_int),
>  	KUNIT_CASE(cmdline_test_tail_int),
>  	KUNIT_CASE(cmdline_test_range),
> +	KUNIT_CASE(cmdline_test_range_overflow),
>  	KUNIT_CASE(cmdline_test_next_arg_quoted_value),
>  	KUNIT_CASE(cmdline_test_next_arg_bare_quote_regression),
>  	KUNIT_CASE(cmdline_test_next_arg_mixed_tokens),
> -- 
> 2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges
       [not found]   ` <CAGD6qbSfTmHq7Y_jM7-QS26at=w6OQwfOZjCU8n4PnnFmqgDNg@mail.gmail.com>
@ 2026-10-07 23:08     ` Andrew Morton
       [not found]       ` <CAGD6qbRJFPX9dYHokumQat3q0AZiG7bgC3hr8rMpCc_BUS6f2g@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2026-10-07 23:08 UTC (permalink / raw)
  To: LZ; +Cc: Armaan Sandhu, Andy Shevchenko, linux-kernel, stable

On Wed, 7 Oct 2026 18:01:08 -0500 LZ <lzsx618@gmail.com> wrote:

> Hi Andrew,
> 
> I came across this issue independently while using ASan and UBSan to look
> for potential vulnerabilities in the Linux kernel. I do not know Armaan and
> have not been in contact with him, so there was no coordination between our
> submissions. The timing appears to be coincidental.
> 
> Regarding the KUnit changes, both patches add a test function named
> cmdline_test_range_overflow, but the actual test cases are different.
> 
> My fix focuses on preventing the index overflow in get_options(). Armaan's
> patch additionally handles overflow in the range-length subtraction and
> corrects the count when a range exceeds the output array's remaining
> capacity.

OK, thanks.

This is the weirdest thing!

> Based on those differences, Armaan's patch appears to address a broader set
> of related issues. Using his implementation as the basis, while retaining
> any complementary regression tests from my patch, seems reasonable to me.

Great.  Can I add your Reviewed-by to Armaan's patch?

Armaan, can you please check the regression tests, see if there's
anything to be incorporated into yours?



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges
       [not found]         ` <CANVJMCFQr9r5-_z3MKkX3G56DWBQTBK8XHW8+YBmme4=F+Cb5A@mail.gmail.com>
@ 2026-10-08  0:08           ` Andrew Morton
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-10-08  0:08 UTC (permalink / raw)
  To: Armaan Sandhu; +Cc: LZ, Andy Shevchenko, linux-kernel, stable

On Wed, 7 Oct 2026 19:50:38 -0400 Armaan Sandhu <armaan.sandhu0504@gmail.com> wrote:

> On Wed, 7 Oct 2026 Andrew Morton <akpm@linux-foundation.org> wrote:
> > Armaan, can you please check the regression tests, see if there's
> > anything to be incorporated into yours?
> 
> Leizhen's two cases pass with my patch. "0-2147483647" in validation
> mode is already in my table, but "1,0-2147483647" isn't, and it is worth
> adding since it hits the overflow guard with a value already parsed, which
> none of my cases do. It's one extra row:
> 
> { "1,0-2147483647", { 3, 1, 0, 1, }, 1, },
> 
> Earlier you asked what would change in a v2. Nothing else, so I can
> send a v2 with that row, or you can fold it in, whichever is easier.
> 
> Thanks Leizhen for the review.

Yes, thanks both.  I queued Armaan's patch and appended this:

--- a/lib/tests/cmdline_kunit.c~lib-cmdline-fix-get_options-count-and-overflow-with-large-ranges-fix
+++ a/lib/tests/cmdline_kunit.c
@@ -151,6 +151,7 @@ static const struct {
 	{ "0-2147483647",  { 3, 0, 1, 2, },    0,          },
 	{ "-5-2147483647", { 3, -5, -4, -3, }, 0,          },
 	{ "2147483647--5", { 0, 2147483647, }, 0,          },
+	{ "1,0-2147483647", { 3, 1, 0, 1, },   1, },
 };
 
 static void cmdline_test_range_overflow(struct kunit *test)
_


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-08  0:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05 21:39 [PATCH] lib/cmdline: fix get_options() count and overflow with large ranges Armaan Sandhu
2026-10-07 22:00 ` Andrew Morton
     [not found]   ` <CAGD6qbSfTmHq7Y_jM7-QS26at=w6OQwfOZjCU8n4PnnFmqgDNg@mail.gmail.com>
2026-10-07 23:08     ` Andrew Morton
     [not found]       ` <CAGD6qbRJFPX9dYHokumQat3q0AZiG7bgC3hr8rMpCc_BUS6f2g@mail.gmail.com>
     [not found]         ` <CANVJMCFQr9r5-_z3MKkX3G56DWBQTBK8XHW8+YBmme4=F+Cb5A@mail.gmail.com>
2026-10-08  0:08           ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®