Thread (15 messages) flat view 15 messages, 3 authors, 20d ago

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

From: sashiko-bot@kernel.org
Date: 2026-08-27 18:47:16

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help