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

[sashiko-bot@kernel.org]

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