[PATCH v2 bpf-next 0/5] Fix several issues in test_progs.c

[PATCH v2 bpf-next 0/5] Fix several issues in test_progs.c

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

Fix several issues in test_progs.c

v2: Fix several issues raised by sashiko-bot

v1: https://lore.kernel.org/all/20260721094404.593127-1-yangfeng59949@163.com/

Feng Yang (5):
  selftests/bpf: Fix double free of subtest_state->name
  selftests/bpf: Fix missing allocation null checks in test_progs.c
  selftests/bpf: Use calloc to allocate subtest_states
  selftests/bpf: Fix memory leak on subtest_states reallocation
  selftests/bpf: Fix potential NULL pointer dereference in strscpy call

 tools/testing/selftests/bpf/test_progs.c | 50 ++++++++++++++++++------
 1 file changed, 39 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH v2 bpf-next 1/5] selftests/bpf: Fix double free of subtest_state->name

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

The name has already been freed in the free_subtest_state function
and does not need to be freed again.

Fixes: 0925225956bb ("bpf/selftests: Add granular subtest output for prog_test")
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/test_progs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 7ba82974ee78..1d3caf996971 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1886,7 +1886,6 @@ static int worker_main_send_subtests(int sock, struct test_state *state)
 			worker_main_send_log(sock, subtest_state->log_buf, subtest_state->log_cnt);
 
 		free_subtest_state(subtest_state);
-		free(subtest_state->name);
 	}
 
 out:
-- 
2.43.0

[PATCH v2 bpf-next 2/5] selftests/bpf: Fix missing allocation null checks in test_progs.c

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

Add null checks after memory allocations to prevent potential segmentation faults.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/test_progs.c | 33 ++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 1d3caf996971..88c65ce5b018 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -730,11 +730,14 @@ int compare_map_keys(int map1_fd, int map2_fd)
 int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
 {
 	__u32 key, next_key, *cur_key_p, *next_key_p;
-	char *val_buf1, *val_buf2;
-	int i, err = 0;
+	char *val_buf1 = NULL, *val_buf2 = NULL;
+	int i, err = -ENOMEM;
 
 	val_buf1 = malloc(stack_trace_len);
 	val_buf2 = malloc(stack_trace_len);
+	if (!val_buf1 || !val_buf2)
+		goto out;
+	err = 0;
 	cur_key_p = NULL;
 	next_key_p = &key;
 	while (bpf_map_get_next_key(smap_fd, cur_key_p, next_key_p) == 0) {
@@ -1514,6 +1517,8 @@ static int dispatch_thread_send_subtests(int sock_fd, struct test_state *state)
 	int subtest_num = state->subtest_num;
 
 	state->subtest_states = malloc(subtest_num * sizeof(*subtest_state));
+	if (!state->subtest_states)
+		return -ENOMEM;
 
 	for (int i = 0; i < subtest_num; i++) {
 		subtest_state = &state->subtest_states[i];
@@ -1732,9 +1737,24 @@ static void server_main(void)
 	sigaction(SIGINT, &sigact_int, NULL);
 
 	dispatcher_threads = calloc(sizeof(pthread_t), env.workers);
+	if (!dispatcher_threads) {
+		perror("Failed to calloc dispatcher_threads");
+		exit(EXIT_ERR_SETUP_INFRA);
+	}
 	data = calloc(sizeof(struct dispatch_data), env.workers);
+	if (!data) {
+		perror("Failed to calloc data");
+		free(dispatcher_threads);
+		exit(EXIT_ERR_SETUP_INFRA);
+	}
 
 	env.worker_current_test = calloc(sizeof(int), env.workers);
+	if (!env.worker_current_test) {
+		perror("Failed to calloc env.worker_current_test");
+		free(data);
+		free(dispatcher_threads);
+		exit(EXIT_ERR_SETUP_INFRA);
+	}
 	for (i = 0; i < env.workers; i++) {
 		int rc;
 
@@ -2094,7 +2114,16 @@ int main(int argc, char **argv)
 	env.worker_id = -1; /* main process */
 	if (env.workers) {
 		env.worker_pids = calloc(sizeof(pid_t), env.workers);
+		if (!env.worker_pids) {
+			perror("Failed to calloc worker_pids");
+			return -ENOMEM;
+		}
 		env.worker_socks = calloc(sizeof(int), env.workers);
+		if (!env.worker_socks) {
+			perror("Failed to calloc worker_socks");
+			free(env.worker_pids);
+			return -ENOMEM;
+		}
 		if (env.debug)
 			fprintf(stdout, "Launching %d workers.\n", env.workers);
 		for (i = 0; i < env.workers; i++) {
-- 
2.43.0

[PATCH v2 bpf-next 3/5] selftests/bpf: Use calloc to allocate subtest_states

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

An early return triggered by read_prog_test_msg leaves uninitialized elements,
which leads to memory corruption during free_test_states cleanup.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/test_progs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 88c65ce5b018..ed9ad7ad3352 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1516,15 +1516,13 @@ static int dispatch_thread_send_subtests(int sock_fd, struct test_state *state)
 	struct subtest_state *subtest_state;
 	int subtest_num = state->subtest_num;
 
-	state->subtest_states = malloc(subtest_num * sizeof(*subtest_state));
+	state->subtest_states = calloc(subtest_num, sizeof(*subtest_state));
 	if (!state->subtest_states)
 		return -ENOMEM;
 
 	for (int i = 0; i < subtest_num; i++) {
 		subtest_state = &state->subtest_states[i];
 
-		memset(subtest_state, 0, sizeof(*subtest_state));
-
 		if (read_prog_test_msg(sock_fd, &msg, MSG_SUBTEST_DONE))
 			return 1;
 
-- 
2.43.0

[PATCH v2 bpf-next 4/5] selftests/bpf: Fix memory leak on subtest_states reallocation

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

Fix memory leak in subtest_states reallocation,
and revert subtest_num if allocation fails.

Fixes: 0925225956bb ("bpf/selftests: Add granular subtest output for prog_test")
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/test_progs.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index ed9ad7ad3352..ea9c887201ae 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -573,18 +573,19 @@ bool test__start_subtest_with_desc(const char *subtest_name, const char *subtest
 	struct subtest_state *subtest_state;
 	const char *subtest_display_name;
 	size_t sub_state_size = sizeof(*subtest_state);
+	void *tmp;
 
 	if (env.subtest_state)
 		test__end_subtest();
 
 	state->subtest_num++;
-	state->subtest_states =
-		realloc(state->subtest_states,
-			state->subtest_num * sub_state_size);
-	if (!state->subtest_states) {
+	tmp = realloc(state->subtest_states, state->subtest_num * sub_state_size);
+	if (!tmp) {
+		state->subtest_num--;
 		fprintf(stderr, "Not enough memory to allocate subtest result\n");
 		return false;
 	}
+	state->subtest_states = tmp;
 
 	subtest_state = &state->subtest_states[state->subtest_num - 1];
 
-- 
2.43.0

[PATCH v2 bpf-next 5/5] selftests/bpf: Fix potential NULL pointer dereference in strscpy call

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

In test__start_subtest_with_desc, if strdup fails or an empty name is passed,
the function returns early. Later, worker_main_send_subtests copies the name via strscpy,
which leads to a NULL pointer dereference.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/test_progs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index ea9c887201ae..ab88268db9e2 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1882,7 +1882,8 @@ static int worker_main_send_subtests(int sock, struct test_state *state)
 
 		msg.subtest_done.num = i;
 
-		strscpy(msg.subtest_done.name, subtest_state->name, MAX_SUBTEST_NAME);
+		const char *name = subtest_state->name ? : "";
+		strscpy(msg.subtest_done.name, name, MAX_SUBTEST_NAME);
 
 		msg.subtest_done.error_cnt = subtest_state->error_cnt;
 		msg.subtest_done.skipped = subtest_state->skipped;
-- 
2.43.0

Re: [PATCH v2 bpf-next 5/5] selftests/bpf: Fix potential NULL pointer dereference in strscpy call

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The patch prevents a NULL pointer dereference in the worker thread by replacing a NULL `subtest_state->name` with an empty string, but misses other instances where a NULL `subtest_state->name` causes undefined behavior or crashes (e.g., in single-process mode and server thread IPC parsing).
--

--- Patch [5]: [PATCH v2 bpf-next 5/5] selftests/bpf: Fix potential NULL pointer dereference in strscpy call ---
commit b6c49e61911a21bc238d913cde0b42bf364476d1
Author: Feng Yang <yangfeng@kylinos.cn>

selftests/bpf: Fix potential NULL pointer dereference in strscpy call

In test__start_subtest_with_desc, if strdup fails or an empty name is passed,
the function returns early. Later, worker_main_send_subtests copies the name
via strscpy, which leads to a NULL pointer dereference.

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index ea9c887201aee..ab88268db9e2e 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1882,7 +1882,8 @@ static int worker_main_send_subtests(int sock, struct test_state *state)
>  
>  		msg.subtest_done.num = i;
>  
> -		strscpy(msg.subtest_done.name, subtest_state->name, MAX_SUBTEST_NAME);
> +		const char *name = subtest_state->name ? : "";
> +		strscpy(msg.subtest_done.name, name, MAX_SUBTEST_NAME);

[Severity: Medium]
This isn't a bug introduced by this patch, but since we are handling a NULL
subtest_state->name here, could a NULL name also cause issues in other places?

For example, in single-process mode, print_subtest_name() receives a NULL
subtest_name and passes it directly to fprintf() as a %s format argument,
triggering undefined behavior:

tools/testing/selftests/bpf/test_progs.c:print_subtest_name() {
    ...
	fprintf(env.stdout_saved, "#%-*s %s/%s",
		TEST_NUM_WIDTH, test_num_str,
		test_name, subtest_name);
    ...
}

When JSON output is requested, dump_test_log() passes it to the JSON writer:

tools/testing/selftests/bpf/test_progs.c:dump_test_log() {
    ...
			jsonw_string_field(w, "name", subtest_state->name);
    ...
}

This leads to a crash when jsonw_puts() dereferences it unconditionally:

tools/bpf/bpftool/json_writer.c:jsonw_puts() {
    ...
	for (; *str; ++str)
    ...
}

Also, could a strdup() failure in dispatch_thread_send_subtests() lead to
subtest_state->name being NULL again on the server side?

tools/testing/selftests/bpf/test_progs.c:dispatch_thread_send_subtests() {
    ...
		subtest_state->name = strdup(msg.subtest_done.name);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722074748.674080-1-yangfeng59949@163.com?part=5

Re: [PATCH v2 bpf-next 2/5] selftests/bpf: Fix missing allocation null checks in test_progs.c

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The newly added NULL check prevents an immediate crash but introduces a segmentation fault during program teardown due to inconsistent state.
- [Medium] Direct return on allocation failure in `main()` and `server_main()` bypasses the cleanup path, leaving the test kernel module loaded.
--

--- Patch [2]: [PATCH v2 bpf-next 2/5] selftests/bpf: Fix missing allocation null checks in test_progs.c ---
commit 5b8390b4a3f20a060d50a407c4941f4877ed4709
Author: Feng Yang <yangfeng@kylinos.cn>

selftests/bpf: Fix missing allocation null checks in test_progs.c
    
Add null checks after memory allocations to prevent potential segmentation faults.

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 1d3caf9969717..88c65ce5b0189 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
[ ... ]
> @@ -1514,6 +1517,8 @@ static int dispatch_thread_send_subtests(int sock_fd, struct test_state *state)
>  	int subtest_num = state->subtest_num;
>  
>  	state->subtest_states = malloc(subtest_num * sizeof(*subtest_state));
> +	if (!state->subtest_states)
> +		return -ENOMEM;

[Severity: Medium]
Will this lead to a segmentation fault during program teardown?

If this allocation fails, we return without clearing state->subtest_num.
Later during teardown, free_test_states() loops over test_state->subtest_num
and accesses test_state->subtest_states[j], which calculates an offset from
a NULL array:

tools/testing/selftests/bpf/test_progs.c:free_test_states() {
    ...
		for (j = 0; j < test_state->subtest_num; j++)
			free_subtest_state(&test_state->subtest_states[j]);
    ...
}

Should state->subtest_num be set to 0 before returning here?

[ ... ]
> @@ -1732,9 +1737,24 @@ static void server_main(void)
>  	sigaction(SIGINT, &sigact_int, NULL);
>  
>  	dispatcher_threads = calloc(sizeof(pthread_t), env.workers);
> +	if (!dispatcher_threads) {
> +		perror("Failed to calloc dispatcher_threads");
> +		exit(EXIT_ERR_SETUP_INFRA);
> +	}

[Severity: Medium]
Does exiting directly here leak the loaded bpf_testmod kernel module?

Normally, main() handles cleanup by going to the out label and calling
unload_bpf_testmod(), but calling exit() bypasses this cleanup path.

[ ... ]
> @@ -2094,7 +2114,16 @@ int main(int argc, char **argv)
>  	env.worker_id = -1; /* main process */
>  	if (env.workers) {
>  		env.worker_pids = calloc(sizeof(pid_t), env.workers);
> +		if (!env.worker_pids) {
> +			perror("Failed to calloc worker_pids");
> +			return -ENOMEM;
> +		}
>  		env.worker_socks = calloc(sizeof(int), env.workers);
> +		if (!env.worker_socks) {
> +			perror("Failed to calloc worker_socks");
> +			free(env.worker_pids);
> +			return -ENOMEM;
> +		}

[Severity: Medium]
Could this direct return also bypass the cleanup path?

Returning directly skips the out label at the end of main(), which is
responsible for unloading the bpf_testmod module if it was loaded. Should this
use a goto to reach the cleanup code?

>  		if (env.debug)
>  			fprintf(stdout, "Launching %d workers.\n", env.workers);

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

Re: [PATCH v2 bpf-next 2/5] selftests/bpf: Fix missing allocation null checks in test_progs.c

[bot+bpf-ci]

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

>     selftests/bpf: Fix missing allocation null checks in test_progs.c
>
>     Add null checks after memory allocations to prevent potential segmentation faults.
>
>     Signed-off-by: Feng Yang <yangfeng@kylinos.cn>

This looks like a bug fix, but there is no Fixes: tag.  Most of the
corrected allocation sites (dispatcher_threads, data, and
env.worker_current_test in server_main(), and env.worker_pids and
env.worker_socks in main()) were added without null checks when the
parallel worker infrastructure went in.  Should this carry:

  Fixes: 91b2c0afd00c ("selftests/bpf: Add parallelism to test_progs")

The change also touches two allocation sites from other commits, the
val_buf1/val_buf2 checks in compare_stack_ips() and the
state->subtest_states check in dispatch_thread_send_subtests(), so more
than one Fixes: tag may be warranted, but 91b2c0afd00c covers the bulk
of the change.


---
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/29903681308