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 <[email protected]> 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/[email protected]?part=3
