Re: [PATCH v5 3/6] lib/string_kunit: Add test for copy_task_comm()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling `set_task_comm()` with a zero-initialized dummy `task_struct` will cause a kernel panic if tracepoints or perf events are enabled, due to a NULL pointer dereference on `task->signal`.
- [Low] The test uses an unexported kernel symbol (`__set_task_comm`), which will cause a linker failure when the KUnit test is compiled as a module.
--

commit 6dcb08d5f95331fc252e0238ecf8e3db7d303fb5
Author: André Almeida <andrealmeid@igalia.com>

lib/string_kunit: Add test for copy_task_comm()

This commit adds a new KUnit test for copy_task_comm() to verify copying from
a task_struct works correctly. It tests the standard case as well as special
cases where the source and destination buffer sizes mismatch.

> diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
> index 0819ace5b0275..27a27ab13254a 100644
> --- a/lib/tests/string_kunit.c
> +++ b/lib/tests/string_kunit.c

[ ... ]

> @@ -881,6 +881,43 @@ static void string_bench_strrchr(struct kunit *test)
>  	STRING_BENCH_BUF(test, buf, len, strrchr, buf, '\0');
>  }
>  
> +#define TASK_NAME "task_name"
> +#define TASK_NAME_LEN 9
> +#define TASK_MAX_LEN TASK_COMM_LEN
> +#define SMALLER_LEN (TASK_NAME_LEN - 3)
> +#define BIGGER_LEN (TASK_MAX_LEN + 3)
> +
> +static void string_copy_task_comm(struct kunit *test)
> +{
> +	char str[TASK_MAX_LEN] = TASK_NAME, copy[TASK_MAX_LEN],
> +	     smaller_buf[SMALLER_LEN], bigger_buf[BIGGER_LEN];
> +	static struct task_struct task, *tsk = &task;
> +	int len1, len2, i;
> +
> +	/* set and get task name */
> +	set_task_comm(tsk, str);

[Severity: High]
Will this cause a kernel panic if tracepoints or perf events are enabled
during the test?

The test uses a zero-initialized static task_struct. The set_task_comm()
macro invokes __set_task_comm(), which eventually calls trace_task_rename()
and perf_event_comm().

If the task_rename tracepoint is enabled, the tracepoint macro dereferences
tsk->signal->oom_score_adj. Similarly, if COMM perf events are active,
perf_event_comm() calls perf_event_pid() which dereferences
tsk->signal->pids[PIDTYPE_TGID].

Since the dummy task's signal pointer is NULL, won't either path crash
the kernel with a NULL pointer dereference?

[Severity: Low]
Does this introduce a build failure when the test is compiled as a module?

CONFIG_STRING_KUNIT_TEST can be built as a module. The set_task_comm() macro
calls __set_task_comm(), which is defined in fs/exec.c but does not appear
to be exported via EXPORT_SYMBOL. 

Will this result in a linker failure at the modpost stage for module builds?

> +	copy_task_comm(copy, tsk, TASK_COMM_LEN);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-tonyk-long_name-v5-0-5fa843782a00@igalia.com?part=3