From: Jiayuan Chen jiayuan.chen@linux.dev
[ Upstream commit f85981327a90c51e76f60e073cb6648b2f167226 ]
The loop in bench_sockmap_prog_destroy() has two issues:
1. Using 'sizeof(ctx.fds)' as the loop bound results in the number of bytes, not the number of file descriptors, causing the loop to iterate far more times than intended.
2. The condition 'ctx.fds[0] > 0' incorrectly checks only the first fd for all iterations, potentially leaving file descriptors unclosed. Change it to 'ctx.fds[i] > 0' to check each fd properly.
These fixes ensure correct cleanup of all file descriptors when the benchmark exits.
Reported-by: Dan Carpenter dan.carpenter@linaro.org Signed-off-by: Jiayuan Chen jiayuan.chen@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Link: https://lore.kernel.org/bpf/20250909124721.191555-1-jiayuan.chen@linux.dev
Closes: https://lore.kernel.org/bpf/aLqfWuRR9R_KTe5e@stanley.mountain/ Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES - `tools/testing/selftests/bpf/benchs/bench_sockmap.c:13` now pulls in `bpf_util.h`, matching the pattern already used by other BPF bench tests so the new `ARRAY_SIZE()` usage compiles on every branch that carries this benchmark (introduced in `7b2fa44de5e71`, tagged around v6.16-rc1). - `tools/testing/selftests/bpf/benchs/bench_sockmap.c:129` replaces the byte-counted `sizeof(ctx.fds)` loop bound with `ARRAY_SIZE(ctx.fds)`, stopping the loop after the five real descriptors instead of wandering into the struct’s counters and repeatedly closing fd 0 or large garbage values. That out-of-bounds iteration currently kills the test’s own stdin and can hand later socket allocations fd 0, so the cleanup path leaks every other socket. - `tools/testing/selftests/bpf/benchs/bench_sockmap.c:130` now checks `ctx.fds[i] > 0` per element instead of reusing `ctx.fds[0]`, which fixes real leak scenarios when the first slot is zero (either after the stray `close(0)` above or when `create_pair()` fails before assigning `c1` but other sockets were opened). - Fix stays confined to the selftest helper and mirrors existing bench code practices, so regression risk is negligible while restoring reliable cleanup for the new sockmap benchmark—exactly the sort of correctness fix stable trees keep so their shipped selftests actually work.
Natural next step: queue this for the stable branches that already picked up `bench_sockmap.c` (v6.16+).
tools/testing/selftests/bpf/benchs/bench_sockmap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/benchs/bench_sockmap.c b/tools/testing/selftests/bpf/benchs/bench_sockmap.c index 8ebf563a67a2b..cfc072aa7fff7 100644 --- a/tools/testing/selftests/bpf/benchs/bench_sockmap.c +++ b/tools/testing/selftests/bpf/benchs/bench_sockmap.c @@ -10,6 +10,7 @@ #include <argp.h> #include "bench.h" #include "bench_sockmap_prog.skel.h" +#include "bpf_util.h"
#define FILE_SIZE (128 * 1024) #define DATA_REPEAT_SIZE 10 @@ -124,8 +125,8 @@ static void bench_sockmap_prog_destroy(void) { int i;
- for (i = 0; i < sizeof(ctx.fds); i++) { - if (ctx.fds[0] > 0) + for (i = 0; i < ARRAY_SIZE(ctx.fds); i++) { + if (ctx.fds[i] > 0) close(ctx.fds[i]); }