Re: [PATCH v5 4/6] sched: Extend task command name with TASK_COMM_EXT_LEN
From: sashiko-bot@kernel.org
Date: 2026-08-27 19:05:02
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] blktrace binary API breakage due to increased payload size for BLK_TN_PROCESS events. - [Medium] Audit logs expose the full 64-byte extended task name, violating the commit message's claim to cap all existing userspace APIs to 16 bytes. - [Medium] The tracepoint `bpf_testmod_test_read` copies the task name without guaranteeing NUL termination, leading to an out-of-bounds read when printing. - [Low] The patch series removed the `get_task_comm` macro but failed to update its callers in the Smack LSM, resulting in an implicit function declaration and a compilation error. -- Hi, I have some questions about this patch. commit 4fa4b0c6a1f339946bd26a6d2c55d7f5cfb4c1f3 Author: André Almeida [off-list ref] Subject: sched: Extend task command name with TASK_COMM_EXT_LEN The commit increases the task command name from 16 to 64 bytes. It aims to support long names for userspace threads while capping existing userspace APIs to TASK_COMM_LEN to avoid buffer overflows.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/sched.h b/include/linux/sched.h index c7cdbb3e4aa68..c41d61e9babf9 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1187,7 +1188,7 @@ struct task_struct { * - set it with set_task_comm() to ensure it is always * NUL-terminated and zero-padded */ - char comm[TASK_COMM_LEN]; + char comm[TASK_COMM_EXT_LEN]; struct nameidata *nameidata;
[Severity: Critical]
With the struct task_struct comm field expanding to 64 bytes, does this break
the blktrace binary ABI?
In kernel/trace/blktrace.c:trace_note_tsk():
trace_note(bt, tsk->pid, BLK_TN_PROCESS, tsk->comm,
sizeof(tsk->comm), 0);
This will now write 64 bytes to the UAPI trace buffer instead of 16 bytes,
which can cause parsing errors or buffer overflows in userspace tools like
blkparse.
[Severity: Medium]
Will out-of-bounds reads happen in existing tracepoints that copy directly
from task->comm using TASK_COMM_LEN?
For example, in tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h:
memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
If a task name is 16 bytes or longer, there won't be a NUL terminator in
the first 16 bytes, which might cause TP_printk("%s") to read past the 16
bytes into adjacent trace event fields.
[ ... ]-#define get_task_comm(buf, tsk) ({ \
- BUILD_BUG_ON(sizeof(buf) < TASK_COMM_LEN); \
- strscpy_pad(buf, (tsk)->comm); \
- buf; \
-})[Severity: Low] Does removing get_task_comm() break the build if CONFIG_SECURITY_SMACK=y? Callers in security/smack/smack_access.c:smack_log() and security/smack/smack_lsm.c:smk_bu_current() still seem to use this macro.
quoted hunk ↗ jump to hunk
diff --git a/kernel/audit.c b/kernel/audit.c index 9412af9144bc9..75bcc0d43c558 100644 --- a/kernel/audit.c +++ b/kernel/audit.c@@ -1667,7 +1667,8 @@ static void audit_log_multicast(int group, const char *op, int err) audit_put_tty(tty); audit_log_task_context(ab); /* subj= */ audit_log_format(ab, " comm="); - audit_log_untrustedstring(ab, get_task_comm(comm, current)); + strscpy_pad(comm, current->comm); + audit_log_untrustedstring(ab, comm);
[Severity: Medium] Since comm is declared as char comm[sizeof(current->comm)] at the top of this function, and current->comm is now 64 bytes, does this expose the full 64-byte task name in the audit log? The commit message mentions capping all existing userspace APIs to TASK_COMM_LEN. This same pattern appears in kernel/audit.c:audit_log_task_info(), kernel/auditsc.c:audit_log_task(), and security/lsm_audit.c:dump_common_audit_data(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827-tonyk-long_name-v5-0-5fa843782a00@igalia.com?part=4