[PATCH bpf v2 0/2] bpf: Fix queue/stack map u32 index overflow

[PATCH bpf v2 0/2] bpf: Fix queue/stack map u32 index overflow

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The queue/stack map addresses elements[] with the product of a u32
head/tail index and value_size, but the storage itself is allocated in
64-bit arithmetic. When max_entries * value_size exceeds U32_MAX, the
product wraps and push/peek/pop operate on the wrong element. Patch 1
restores the dropped bound check; patch 2 adds a regression test.

Changes in v2:
  - Also reject max_entries == U32_MAX: the u32 capacity counter
    qs->size (max_entries + 1) would wrap to 0 and permanently break
    the map, as pointed out in review.
  - Fix the multi-line comment style in the selftest, as pointed out in
    review.

Yuan Chen (2):
  bpf: Fix queue/stack map u32 index overflow
  selftests/bpf: Add regression test for queue/stack map size limit

 kernel/bpf/queue_stack_maps.c                 | 10 +++++
 .../bpf/prog_tests/queue_stack_map.c          | 45 +++++++++++++++++++
 2 files changed, 55 insertions(+)

-- 
2.54.0

[PATCH bpf v2 1/2] bpf: Fix queue/stack map u32 index overflow

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The queue/stack map addresses elements[] with the product of a u32
head/tail index and value_size, but the storage itself is allocated in
64-bit arithmetic. When max_entries * value_size exceeds U32_MAX, the
product wraps and push/peek/pop operate on the wrong element, corrupting
map data and leaking stale values to user space.

The original bound check was removed by commit c85d69135a91 ("bpf: move
memory size checks to bpf_map_charge_init()"), which migrated only the
bytes-to-pages conversion and dropped the overflow guard, so oversized
queue/stack maps can be created again.

Restore the bound in queue_stack_map_alloc_check(): reject maps whose
element storage would exceed U32_MAX bytes, keeping the u32 index
multiplication overflow-free. Also reject max_entries == U32_MAX, which
would make the u32 capacity counter qs->size (max_entries + 1) wrap to
0 and permanently break the map.

Fixes: c85d69135a91 ("bpf: move memory size checks to bpf_map_charge_init()")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/queue_stack_maps.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index c1c9dee4dcdd..bb2453693b80 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -59,6 +59,16 @@ static int queue_stack_map_alloc_check(union bpf_attr *attr)
 		 */
 		return -E2BIG;
 
+	/*
+	 * The u32 head/tail index is multiplied by value_size to address
+	 * elements[], and qs->size (max_entries + 1) is stored in a u32.
+	 * Bound the map size so neither the product nor the capacity
+	 * counter can overflow.
+	 */
+	if ((u64)attr->max_entries * attr->value_size > U32_MAX ||
+	    attr->max_entries == U32_MAX)
+		return -E2BIG;
+
 	return 0;
 }
 
-- 
2.54.0

[PATCH bpf v2 2/2] selftests/bpf: Add regression test for queue/stack map size limit

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

Verify that queue/stack maps whose element storage would overflow the
u32 head/tail index multiplication are rejected at creation time, and
that max_entries == U32_MAX (which would wrap the u32 capacity counter
to 0) is rejected as well.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../bpf/prog_tests/queue_stack_map.c          | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
index 41441325e179..8ab07ea77775 100644
--- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
@@ -101,8 +101,53 @@ static void test_queue_stack_map_by_type(int type)
 	bpf_object__close(obj);
 }
 
+static void test_queue_stack_map_alloc_check(void)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts);
+	const __u32 big_value = 1 << 20; /* 1MB */
+	int fd, saved_errno;
+
+	/*
+	 * Regression test for the u32 index overflow in queue/stack maps:
+	 * a map whose element storage (max_entries * value_size) exceeds
+	 * U32_MAX bytes must be rejected at creation time, otherwise the
+	 * u32 head/tail index multiplication wraps and push/peek/pop
+	 * address the wrong element. 8192 * 1MB = 8GB > U32_MAX.
+	 */
+	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, big_value, 8192, &opts);
+	saved_errno = errno;
+	ASSERT_LT(fd, 0, "queue_oversize_fd");
+	ASSERT_EQ(saved_errno, E2BIG, "queue_oversize_errno");
+
+	/*
+	 * max_entries == U32_MAX would make the u32 capacity counter
+	 * qs->size (max_entries + 1) wrap to 0, permanently breaking the
+	 * map, so it must be rejected as well.
+	 */
+	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, 1, U32_MAX, &opts);
+	saved_errno = errno;
+	ASSERT_LT(fd, 0, "queue_u32max_fd");
+	ASSERT_EQ(saved_errno, E2BIG, "queue_u32max_errno");
+	if (fd >= 0)
+		close(fd);
+
+	fd = bpf_map_create(BPF_MAP_TYPE_STACK, NULL, 0, big_value, 8192, &opts);
+	saved_errno = errno;
+	ASSERT_LT(fd, 0, "stack_oversize_fd");
+	ASSERT_EQ(saved_errno, E2BIG, "stack_oversize_errno");
+	if (fd >= 0)
+		close(fd);
+
+	/* A normal-sized map must still be created successfully. */
+	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, 64, 100, &opts);
+	ASSERT_GE(fd, 0, "queue_normal_fd");
+	if (fd >= 0)
+		close(fd);
+}
+
 void test_queue_stack_map(void)
 {
 	test_queue_stack_map_by_type(QUEUE);
 	test_queue_stack_map_by_type(STACK);
+	test_queue_stack_map_alloc_check();
 }
-- 
2.54.0

Re: [PATCH bpf v2 2/2] selftests/bpf: Add regression test for queue/stack map size limit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing file descriptor cleanup on the test failure path for the first bpf_map_create call.
--

--- Patch [2]: [PATCH bpf v2 2/2] selftests/bpf: Add regression test for queue/stack map size limit ---
commit 25c3a9fe96d53115b495649d174f217a4729daca
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Add regression test for queue/stack map size limit

This commit adds regression tests to ensure that queue and stack maps whose
element storage would overflow the u32 head/tail index multiplication or
wrap the u32 capacity counter are properly rejected at creation time.

> diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> index 41441325e1794..8ab07ea777759 100644
> --- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
[ ... ]
> @@ -101,8 +101,53 @@ static void test_queue_stack_map_by_type(int type)
>  	bpf_object__close(obj);
>  }
>  
> +static void test_queue_stack_map_alloc_check(void)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts);
> +	const __u32 big_value = 1 << 20; /* 1MB */
> +	int fd, saved_errno;
> +
> +	/*
> +	 * Regression test for the u32 index overflow in queue/stack maps:
> +	 * a map whose element storage (max_entries * value_size) exceeds
> +	 * U32_MAX bytes must be rejected at creation time, otherwise the
> +	 * u32 head/tail index multiplication wraps and push/peek/pop
> +	 * address the wrong element. 8192 * 1MB = 8GB > U32_MAX.
> +	 */
> +	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, big_value, 8192, &opts);
> +	saved_errno = errno;
> +	ASSERT_LT(fd, 0, "queue_oversize_fd");
> +	ASSERT_EQ(saved_errno, E2BIG, "queue_oversize_errno");

[Severity: Low]
Should this test block include a close() call in case bpf_map_create()
unexpectedly succeeds?

If the kernel fails to reject the oversized map, fd will be a non-negative
value. Since ASSERT_LT logs the test failure but doesn't abort execution,
fd is overwritten in the very next bpf_map_create() call without being
cleaned up.

The subsequent tests in this function include a check to close the file
descriptor on the failure path:

> +
> +	/*
> +	 * max_entries == U32_MAX would make the u32 capacity counter
> +	 * qs->size (max_entries + 1) wrap to 0, permanently breaking the
> +	 * map, so it must be rejected as well.
> +	 */
> +	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, 1, U32_MAX, &opts);
> +	saved_errno = errno;
> +	ASSERT_LT(fd, 0, "queue_u32max_fd");
> +	ASSERT_EQ(saved_errno, E2BIG, "queue_u32max_errno");
> +	if (fd >= 0)
> +		close(fd);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810092814.2698521-1-chenyuan_fl@163.com?part=2

Re: [PATCH bpf v2 2/2] selftests/bpf: Add regression test for queue/stack map size limit

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> index 41441325e179..8ab07ea77775 100644
> --- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> @@ -101,8 +101,53 @@ static void test_queue_stack_map_by_type(int type)
>  	bpf_object__close(obj);
>  }
>
> +static void test_queue_stack_map_alloc_check(void)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts);
> +	const __u32 big_value = 1 << 20; /* 1MB */
> +	int fd, saved_errno;
> +
> +	/*
> +	 * Regression test for the u32 index overflow in queue/stack maps:
> +	 * a map whose element storage (max_entries * value_size) exceeds
> +	 * U32_MAX bytes must be rejected at creation time, otherwise the
> +	 * u32 head/tail index multiplication wraps and push/peek/pop
> +	 * address the wrong element. 8192 * 1MB = 8GB > U32_MAX.
> +	 */
> +	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, big_value, 8192, &opts);
> +	saved_errno = errno;
> +	ASSERT_LT(fd, 0, "queue_oversize_fd");
> +	ASSERT_EQ(saved_errno, E2BIG, "queue_oversize_errno");
> +
> +	/*
> +	 * max_entries == U32_MAX would make the u32 capacity counter
> +	 * qs->size (max_entries + 1) wrap to 0, permanently breaking the
> +	 * map, so it must be rejected as well.
> +	 */
> +	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, 1, U32_MAX, &opts);
                                                                    ^^^^^^^

Does this build? U32_MAX is not defined in any header reachable from this
file.

U32_MAX exists only in include/linux/limits.h in the kernel tree, which is
not part of include/uapi/ and has no copy under tools/include/. The BPF
selftests include closure (test_progs.h and its dependencies) does not
provide this macro. libc provides UINT_MAX and UINT32_MAX, but not U32_MAX.

Existing selftests that need U32_MAX define it locally. In the same
directory, prog_tests/reg_bounds.c does:

  #define U32_MAX ((u32)UINT_MAX)

Since the BPF selftests Makefile builds with -Wall -Werror and every
prog_tests/*.c is linked into the single test_progs binary, this breaks the
entire selftest build rather than just this subtest.

Should this either define the macro locally (as reg_bounds.c does) or use
UINT_MAX from <limits.h>?

> +	saved_errno = errno;
> +	ASSERT_LT(fd, 0, "queue_u32max_fd");
> +	ASSERT_EQ(saved_errno, E2BIG, "queue_u32max_errno");
> +	if (fd >= 0)
> +		close(fd);
> +
> +	fd = bpf_map_create(BPF_MAP_TYPE_STACK, NULL, 0, big_value, 8192, &opts);
> +	saved_errno = errno;
> +	ASSERT_LT(fd, 0, "stack_oversize_fd");
> +	ASSERT_EQ(saved_errno, E2BIG, "stack_oversize_errno");
> +	if (fd >= 0)
> +		close(fd);
> +
> +	/* A normal-sized map must still be created successfully. */
> +	fd = bpf_map_create(BPF_MAP_TYPE_QUEUE, NULL, 0, 64, 100, &opts);
> +	ASSERT_GE(fd, 0, "queue_normal_fd");
> +	if (fd >= 0)
> +		close(fd);
> +}
> +
>  void test_queue_stack_map(void)
>  {
>  	test_queue_stack_map_by_type(QUEUE);
>  	test_queue_stack_map_by_type(STACK);
> +	test_queue_stack_map_alloc_check();
>  }


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31375986836

Re: [PATCH bpf v2 1/2] bpf: Fix queue/stack map u32 index overflow

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3567 bytes --]

>     bpf: Fix queue/stack map u32 index overflow
>
>     The queue/stack map addresses elements[] with the product of a u32
>     head/tail index and value_size, but the storage itself is allocated in
>     64-bit arithmetic. When max_entries * value_size exceeds U32_MAX, the
>     product wraps and push/peek/pop operate on the wrong element, corrupting
>     map data and leaking stale values to user space.
>
>     The original bound check was removed by commit c85d69135a91 ("bpf: move
>     memory size checks to bpf_map_charge_init()"), which migrated only the
>     bytes-to-pages conversion and dropped the overflow guard, so oversized
>     queue/stack maps can be created again.
>
>     Restore the bound in queue_stack_map_alloc_check(): reject maps whose
>     element storage would exceed U32_MAX bytes, keeping the u32 index
>     multiplication overflow-free. Also reject max_entries == U32_MAX, which
>     would make the u32 capacity counter qs->size (max_entries + 1) wrap to
>     0 and permanently break the map.
>
>     Fixes: c85d69135a91 ("bpf: move memory size checks to bpf_map_charge_init()")
>     Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

Is the attribution to c85d69135a91 right here?  The changelog says that
commit "migrated only the bytes-to-pages conversion and dropped the
overflow guard", but it looks like the guard was moved rather than
dropped.

It removed the check from queue_stack_map_alloc():

kernel/bpf/queue_stack_maps.c:queue_stack_map_alloc() {
	cost = queue_size = sizeof(*qs) + size * attr->value_size;
-	if (cost >= U32_MAX - PAGE_SIZE)
-		return ERR_PTR(-E2BIG);
-
-	cost = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;

	ret = bpf_map_charge_init(&mem, cost);
}

and the same commit added the equivalent check inside the callee:

kernel/bpf/syscall.c:bpf_map_charge_init() {
-int bpf_map_charge_init(struct bpf_map_memory *mem, u32 pages)
+int bpf_map_charge_init(struct bpf_map_memory *mem, size_t size)
 {
+	u32 pages = round_up(size, PAGE_SIZE) >> PAGE_SHIFT;
 	...
+	if (size >= U32_MAX - PAGE_SIZE)
+		return -E2BIG;
}

Since queue_stack_map_alloc() kept passing the full
sizeof(*qs) + (max_entries + 1) * value_size as cost, wouldn't oversized
queue and stack maps still have been rejected with -E2BIG right after
c85d69135a91?

The bound instead looks lost in a37fb7ef24a4 ("bpf: Eliminate
rlimit-based memory accounting for queue_stack_maps maps"), which deleted
the charge call entirely:

kernel/bpf/queue_stack_maps.c:queue_stack_map_alloc() {
-	ret = bpf_map_charge_init(&mem, cost);
-	if (ret < 0)
-		return ERR_PTR(ret);

	qs = bpf_map_area_alloc(queue_size, numa_node);
}

That leaves bpf_map_area_alloc() as the only limit on queue_size, and it
only rejects size >= SIZE_MAX on 64-bit.  Should the tag read:

  Fixes: a37fb7ef24a4 ("bpf: Eliminate rlimit-based memory accounting for queue_stack_maps maps")

with the middle paragraph reworded to match?

The difference also changes which trees the fix lands in.  As written the
tag points stable tooling at v5.3 and the v5.3..v5.10 trees, where the
check was still effective through bpf_map_charge_init(), while the
affected range appears to start at v5.11.

Given that the wrong element is addressed on push, peek and pop from
v5.11 onward, would a Cc: stable@vger.kernel.org line be appropriate as
well?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31375986836