Re: [PATCH bpf-next v5 2/2] selftests/bpf: Cover recursive bpftool map dumps

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Dangerous accumulation of `snprintf` return values without bounds checking can lead to a stack buffer overflow.
--

commit 9bcae338c80433d98472777c175bac68e5e0d8b5
Author: Tianyi Chen <hi@tychen.cc>

selftests/bpf: Cover recursive bpftool map dumps

This commit adds selftests to exercise recursive map dumping for
array-of-maps and hash-of-maps. It verifies complete JSON documents against
nonrecursive entry representations, checks both short and long options, and
tests file descriptor retention under limits.

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c
[ ... ]
> +static void test_multiple_roots(void)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts);
> +	char command[MAX_BPFTOOL_CMD_LEN], name[BPF_OBJ_NAME_LEN];
> +	char output[OUTPUT_SIZE] = {}, expected[OUTPUT_SIZE * 4], elements[OUTPUT_SIZE];
[ ... ]
> +	for (i = 0; i < ARRAY_SIZE(fds); i++) {
> +		ids[i] = map_id(fds[i]);
> +		if (!ids[i] || !dump_map(ids[i], "-j", elements))
> +			goto out;
> +		len += snprintf(expected + len, sizeof(expected) - len,
> +				"%s{\"id\":%u,\"type\":\"%s\",\"name\":\"%s\","
> +				"\"flags\":0,\"elements\":%s}%s",
> +				i ? "," : "[", ids[i], types[i],
> +				i == 3 ? "dump_discovered" : name, elements, i == 3 ? "]" : "");

[Severity: Medium]
Could this accumulation of the snprintf return value cause a stack buffer
overflow?

snprintf returns the number of bytes that would have been written, not the
number of bytes actually written. If the accumulated length exceeds
sizeof(expected), the subsequent loop iteration passes sizeof(expected) - len
as the buffer size, which underflows to a large unsigned integer since the
sizeof operator yields a size_t. 

Since the elements buffer can hold up to 8191 bytes, over 4 iterations
this can mathematically exceed the 32768 bytes of expected when combined with
the JSON formatting overhead (for instance, if bpftool produces unexpectedly
long output or errors).

Does this code need a bounds check before adding to len to prevent writing
past the end of the expected stack buffer?

> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911034715.219552-1-diannaaav@gmail.com?part=2