[PATCH 0/6] bitmap: fix three parsing bugs in bitmap_parse() and bitmap_parselist()

[PATCH 0/6] bitmap: fix three parsing bugs in bitmap_parse() and bitmap_parselist()

[shashank]

This series fixes three cases where the bitmap string parsers accept
input they are documented to reject, or produce a different mask than
the one requested.  Each fix is followed by a patch adding the failing
cases to lib/test_bitmap.c.

 1-2: bitmap_parse() treats a non-hex character directly before a chunk
      of exactly eight hex digits as a separator, so "x12345678" and
      "0x0000000f" are accepted while "0xf" is rejected.  This is a
      regression from the bitmap_parse() rework; the old __bitmap_parse()
      rejected all of them.

 3-4: bitmap_parselist() does not check for the end of the region after
      the group size of a "range:used/group" region, so since N and
      all became valid region starts, "0-7:1/2N" or "0-7:1/2all" are
      silently parsed as two regions.

 5-6: bitmap_parselist() walks a "range:used/group" region with an
      unsigned int that wraps around when the group size is close to
      UINT_MAX, setting bits below the requested range.  This needs an
      absurd group size and never writes out of bounds.

With the series applied, test_bitmap reports "all 391544 tests passed"
(UML, x86_64).  The three fixes are independent and can be applied in
any order.

smjain (6):
  bitmap: bitmap_parse(): reject non-hex character before 8 digits
  bitmap: test bitmap_parse() with an illegal character before 8 hex
    digits
  bitmap: bitmap_parselist(): reject trailing characters after group
    size
  bitmap: test bitmap_parselist() with text after the group size
  bitmap: bitmap_parselist(): don't wrap around on a huge group size
  bitmap: test bitmap_parselist() with a group size close to UINT_MAX

 lib/bitmap-str.c  | 24 +++++++++++++++++++-----
 lib/test_bitmap.c |  9 +++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/6] bitmap: bitmap_parse(): reject non-hex character before 8 digits

[shashank]

bitmap_parse() is documented to return -EINVAL for illegal characters,
and it does so everywhere except when the illegal character directly
precedes a chunk of exactly eight hex digits.

bitmap_get_x32_reverse() parses a chunk from right to left.  After it
has consumed eight digits it looks at one more character: a hex digit
means the chunk is wider than 32 bits and -EOVERFLOW is returned.  Any
other character is silently consumed as if it were a separator, and the
remaining input is parsed as the next chunk.  As a result:

  "x12345678"   -> 0x12345678, returns 0 (expected -EINVAL)
  "1g12345678"  -> 0x1_12345678, returns 0 (expected -EINVAL)
  "0x0000000f"  -> 0xf, returns 0, while "0xf" returns -EINVAL

bitmap_parse() backs cpumask_parse() and cpumask_parse_user(), so the
same inconsistency is visible when writing masks such as
/proc/irq/*/smp_affinity or the network queue rps_cpus/xps_cpus files.

__bitmap_parse(), which this function replaced in commit 2d6261583be0
("lib: rework bitmap_parse()"), rejected all three strings with -EINVAL,
and a "0x" prefix is accepted today only when exactly eight digits
follow it, so no reliably working input is lost.

When the loop falls through, *end is known to be a non-separator
character inside the buffer, so return -EOVERFLOW if it is a hex digit
and -EINVAL otherwise.

Fixes: 2d6261583be0 ("lib: rework bitmap_parse()")
Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/bitmap-str.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
index dd9aa0635fa5..e02c5bc80951 100644
--- a/lib/bitmap-str.c
+++ b/lib/bitmap-str.c
@@ -414,8 +414,12 @@ static const char *bitmap_get_x32_reverse(const char *start,
 			goto out;
 	}
 
-	if (hex_to_bin(*end--) >= 0)
-		return ERR_PTR(-EOVERFLOW);
+	/*
+	 * Eight digits have been consumed and the next character is not a
+	 * separator: another hex digit means the chunk does not fit in 32
+	 * bits, anything else is an illegal character.
+	 */
+	return ERR_PTR(hex_to_bin(*end) >= 0 ? -EOVERFLOW : -EINVAL);
 out:
 	*num = ret;
 	return end;
-- 
2.43.0

[PATCH 2/6] bitmap: test bitmap_parse() with an illegal character before 8 hex digits

[shashank]

Add bitmap_parse() cases where an illegal character directly precedes a
chunk of eight hex digits.  These were accepted before the previous
patch and must return -EINVAL.

Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/test_bitmap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 56bd23059b26..35bb18f6e326 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -643,6 +643,10 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
 	{-EOVERFLOW, "badf00d,deadbeef,1,0",	NULL, 90, 0},
 	{-EOVERFLOW, "fbadf00d,deadbeef,1,0",	NULL, 95, 0},
 	{-EOVERFLOW, "badf00d,deadbeef,1,0",	NULL, 100, 0},
+
+	{-EINVAL,    "x12345678",		NULL, 32, 0},
+	{-EINVAL,    "1g12345678",		NULL, 64, 0},
+	{-EINVAL,    "0x0000000f",		NULL, 64, 0},
 #undef step
 };
 
-- 
2.43.0

[PATCH 3/6] bitmap: bitmap_parselist(): reject trailing characters after group size

[shashank]

bitmap_parse_region() checks that every region ends at a separator
(',', whitespace, '\n' or '\0'), except in the "range:used/group" form:
after parsing the group size it returns the pointer to the following
character without looking at it.  The caller then starts a new region
at that character.

Before 'N' and 'all' were accepted as region starts this happened to be
harmless, because any other character would fail to parse as a new
region.  Now text directly following the group size is silently parsed
as another region:

  "0-7:1/2N"    -> bits 0,2,4,6,7 (nbits = 8), returns 0
  "0-7:1/2all"  -> all bits set, returns 0

while "0-7N", "5N" or "0-7:1/2x" are correctly rejected with -EINVAL.
bitmap_parselist() is documented to return -EINVAL for an invalid
character.  It parses cpu lists from sysfs, cgroup cpuset files and
boot parameters such as isolcpus= and nohz_full=, so a typo there is
accepted and yields a different mask than intended.

Check for the end of the region after the group size as well, like the
other forms do.

Fixes: 2c4885d24e64 ("lib: bitmap: support "N" as an alias for size of bitmap")
Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/bitmap-str.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
index e02c5bc80951..b58966864657 100644
--- a/lib/bitmap-str.c
+++ b/lib/bitmap-str.c
@@ -299,7 +299,14 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
 	if (*str != '/')
 		return ERR_PTR(-EINVAL);
 
-	return bitmap_getnum(str + 1, &r->group_len, lastbit);
+	str = bitmap_getnum(str + 1, &r->group_len, lastbit);
+	if (IS_ERR(str))
+		return str;
+
+	if (!end_of_region(*str))
+		return ERR_PTR(-EINVAL);
+
+	return end_of_str(*str) ? NULL : str;
 
 no_end:
 	r->end = r->start;
-- 
2.43.0

[PATCH 4/6] bitmap: test bitmap_parselist() with text after the group size

[shashank]

Add bitmap_parselist() cases where 'N' or 'all' directly follows the
group size of a "range:used/group" region.  These were accepted before
"bitmap: bitmap_parselist(): reject trailing characters after group
size" and must return -EINVAL.

Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/test_bitmap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 35bb18f6e326..f316a82840f5 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -528,6 +528,9 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{-EINVAL, "a-31:10/1", NULL, 8, 0},
 	{-EINVAL, "0-31:a/1", NULL, 8, 0},
 	{-EINVAL, "0-\n", NULL, 8, 0},
+	{-EINVAL, "0-7:1/2N", NULL, 8, 0},
+	{-EINVAL, "0-7:1/2all", NULL, 8, 0},
+	{-EINVAL, "all:1/2N", NULL, 8, 0},
 
 };
 
-- 
2.43.0

[PATCH 5/6] bitmap: bitmap_parselist(): don't wrap around on a huge group size

[shashank]

In the "range:used/group" form bitmap_set_region() walks the range with

	for (start = r->start; start <= r->end; start += r->group_len)

where all values are unsigned int.  bitmap_getnum() accepts any group
size up to UINT_MAX, and bitmap_check_region() only requires it to be
non-zero and at least the used size.  If start + group_len exceeds
UINT_MAX, start wraps around to a small value that is still <= end, and
bits below the start of the range get set:

  "1-1:1/4294967295"    -> bits 0,1 (expected bit 1)
  "15-15:1/4294967281"  -> bits 0,15 (expected bit 15)

The loop still terminates, because start keeps decreasing until it
wraps past zero again, but the resulting mask contains bits outside
the requested range.

This needs a group size within 'start' of UINT_MAX, so it is unlikely
to be hit by accident; all bits set stay below the end of the range,
so there is no out-of-bounds write.

A group larger than the range simply means that the range contains a
single group, so stop the walk when the next group would start beyond
UINT_MAX.

Fixes: 0a5ce0831d04 ("lib/bitmap.c: make bitmap_parselist() thread-safe and much faster")
Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/bitmap-str.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
index b58966864657..cafd6388892f 100644
--- a/lib/bitmap-str.c
+++ b/lib/bitmap-str.c
@@ -8,6 +8,7 @@
 #include <linux/hex.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
+#include <linux/overflow.h>
 #include <linux/string.h>
 
 #include "kstrtox.h"
@@ -186,10 +187,12 @@ struct region {
 
 static void bitmap_set_region(const struct region *r, unsigned long *bitmap)
 {
-	unsigned int start;
+	unsigned int start = r->start;
 
-	for (start = r->start; start <= r->end; start += r->group_len)
+	do {
 		bitmap_set(bitmap, start, min(r->end - start + 1, r->off));
+	} while (!check_add_overflow(start, r->group_len, &start) &&
+		 start <= r->end);
 }
 
 static int bitmap_check_region(const struct region *r)
-- 
2.43.0

[PATCH 6/6] bitmap: test bitmap_parselist() with a group size close to UINT_MAX

[shashank]

Add bitmap_parselist() cases where the group size makes start +
group_len overflow an unsigned int.  Before "bitmap: bitmap_parselist():
don't wrap around on a huge group size" the region walk wrapped around
and also set bit 0.

Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/test_bitmap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index f316a82840f5..ba09d057f47f 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -502,6 +502,8 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, "0-N:1/3,1-N:1/3,2-N:1/3",		&exp1[8 * step], 32, 0},
 	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
 	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
+	{0, "1-1:1/4294967295",			&exp1[1 * step], 32, 0},
+	{0, "15-15:1/4294967281",		&exp1[13 * step], 32, 0},
 
 	{0,	  "all",		&exp1[8 * step], 32, 0},
 	{0,	  "0, 1, all,  ",	&exp1[8 * step], 32, 0},
-- 
2.43.0