* [PATCH v8 0/5] Add support for long task name
@ 2025-08-21 10:21 Bhupesh
2025-08-21 10:21 ` [PATCH v8 1/5] exec: Remove obsolete comments Bhupesh
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
Changes since v7:
================
- v7 can be seen here: https://lore.kernel.org/all/20250811064609.918593-1-bhupesh@igalia.com/
- As suggested by Andy used '--base' option with git-format to indicate the version of
'linux-next/master' this patchset is rebased on. Also used the 'Link'
tag in the commit log.
- Also added a new [PATCH 5/5] to replace BUILD_BUG_ON() with static_assert() inside
'set_task_comm()'.
Changes since v6:
================
- v6 can be seen here: https://lore.kernel.org/all/20250724123612.206110-1-bhupesh@igalia.com/
- As suggested by Linus, we got rid of 'get_task_comm()' entirely and replaced it with
'strscpy_pad()' implementation.
- Also changed the current memcpy() use-cases which use 'current->comm' to copy the task name over
to local copies, to call a wrappper like "get_task_array()".
- Collected 'Reviewed-by' from Kees for [PATCH 1/4].
Changes since v5:
================
- v5 can be seen here: https://lore.kernel.org/lkml/20250716123916.511889-1-bhupesh@igalia.com/
- As suggested by Linus, replaced 'tsk->comm' with 'tsk->comm_str' locally, and verified basic
thread names and then changed 'tsk->comm_str' back to 'tsk->comm'. So essentially now 'tsk->comm'
is TASK_COMM_EXT_LEN i.e. 64-bytes long.
Changes since v4:
================
- v4 can be seen here: https://lore.kernel.org/lkml/20250521062337.53262-1-bhupesh@igalia.com/
- As suggested by Kees, replaced tsk->comm with tsk->comm_str, inside 'task_struct'
where TASK_COMM_EXT_LEN is 64-bytes.
Changes since v3:
================
- v3 can be seen here: https://lore.kernel.org/lkml/20250507110444.963779-1-bhupesh@igalia.com/
- As suggested by Petr and Steven, used 'comm_ext' name instead of
'real_comm'. Correspondingly the macro name is changed to 'TASK_COMM_EXT_LEN'
for the 64-byte extended comm.
- Rebased this patchset on linux-next/master, which contain the following patch from
Steven now:
155fd6c3e2f0 ("tracing/sched: Use __string() instead of fixed lengths for task->comm")
- Accordingly, v4 drops the changes done for 'trace/sched' events in v3,
but retains the 'safe' memcpy' changes for other kernel trace users.
Changes since v2:
================
- v2 can be seen here: https://lore.kernel.org/lkml/20250331121820.455916-1-bhupesh@igalia.com/
- As suggested by Yafang and Kees, picked Linus' suggested approach for
this version (see: <https://lore.kernel.org/all/CAHk-=wjAmmHUg6vho1KjzQi2=psR30+CogFd4aXrThr2gsiS4g@mail.gmail.com/>).
- Dropped kthreads patch from this version. It would be sent out
separately, if we have a consensus on this approach.
Changes since v1:
================
- v1 can be seen here: https://lore.kernel.org/lkml/20250314052715.610377-1-bhupesh@igalia.com/
- As suggested by Kees, added [PATCH 3/3] to have a consistent
'full_name' entry inside 'task_struct' which both tasks and
kthreads can use.
- Fixed the commit message to indicate that the existing ABI
'/proc/$pid/task/$tid/comm' remains untouched and a parallel
'/proc/$pid/task/$tid/full_name' ABI for new (interested) users.
While working with user-space debugging tools which work especially
on linux gaming platforms, I found that the task name is truncated due
to the limitation of TASK_COMM_LEN.
Now, during debug tracing, seeing truncated names is not very useful,
especially on gaming platforms where the number of tasks running can
be very high.
This patchset does not touch 'TASK_COMM_LEN' at all, i.e.
'TASK_COMM_LEN' and the 16-byte design remains untouched.
Via this patchset, Linus suggested 'tsk->comm' is made 64-byte long
and equal to TASK_COMM_EXT_LEN.
To avoid any surprises / bug,s I replaced 'tsk->comm' with
'tsk->comm_str' locally inside 'task_struct' and checked compilation
of code and basic working of thread names:
struct task_struct {
..............
char comm_str[TASK_COMM_EXT_LEN];
..............
};
where TASK_COMM_EXT_LEN is 64-bytes.
Once done, I changed the name back to 'tsk->comm'.
To ensure that the existing ABI and userspace continues to work
as intended, we ensure that:
- Existing users of 'get_task_comm'/ 'set_task_comm' will get 'tsk->comm'
truncated to a maximum of 'TASK_COMM_LEN' (16-bytes) to maintain ABI,
- New / Modified users of 'get_task_comm'/ 'set_task_comm' will get
'tsk->comm' supported up to a maximum of 'TASK_COMM_EXT_LEN' (64-bytes).
Note, that the existing users have not been modified to migrate to
'TASK_COMM_EXT_LEN', in case they have hard-coded expectations of
dealing with only a 'TASK_COMM_LEN' long 'tsk->comm_str'.
After this change, gdb is able to show full name of the task, using a
simple app which generates threads with long names [see 1]:
# gdb ./threadnames -ex "run info thread" -ex "detach" -ex "quit" > log
# cat log
NameThatIsTooLongForComm[4662]
[1]. https://github.com/lostgoat/tasknames
Bhupesh (5):
exec: Remove obsolete comments
include: Set tsk->comm length to 64 bytes
treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
treewide: Switch memcpy() users of 'task->comm' to a more safer
implementation
include: Replace BUILD_BUG_ON with static_assert in 'set_task_comm()'
drivers/connector/cn_proc.c | 2 +-
drivers/dma-buf/sw_sync.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c | 2 +-
.../drm/amd/amdgpu/amdgpu_eviction_fence.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 +-
drivers/gpu/drm/lima/lima_ctx.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
drivers/gpu/drm/panthor/panthor_gem.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
drivers/hwtracing/stm/core.c | 2 +-
drivers/tty/tty_audit.c | 2 +-
fs/bcachefs/thread_with_file.c | 2 +-
fs/binfmt_elf.c | 2 +-
fs/binfmt_elf_fdpic.c | 2 +-
fs/ocfs2/cluster/netdebug.c | 1 -
fs/proc/array.c | 2 +-
include/linux/coredump.h | 2 +-
include/linux/sched.h | 51 ++++++++++++-------
include/linux/tracepoint.h | 4 +-
include/trace/events/block.h | 10 ++--
include/trace/events/oom.h | 2 +-
include/trace/events/osnoise.h | 2 +-
include/trace/events/sched.h | 13 +++--
include/trace/events/signal.h | 2 +-
include/trace/events/task.h | 4 +-
kernel/audit.c | 6 ++-
kernel/auditsc.c | 6 ++-
kernel/sys.c | 2 +-
mm/kmemleak.c | 6 ---
net/bluetooth/hci_sock.c | 2 +-
net/netfilter/nf_tables_api.c | 2 +-
security/integrity/integrity_audit.c | 3 +-
security/ipe/audit.c | 2 +-
security/landlock/domain.c | 2 +-
security/lsm_audit.c | 7 +--
tools/bpf/bpftool/pids.c | 6 +--
.../bpf/test_kmods/bpf_testmod-events.h | 2 +-
40 files changed, 92 insertions(+), 83 deletions(-)
base-commit: 5303936d609e09665deda94eaedf26a0e5c3a087
--
2.38.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 1/5] exec: Remove obsolete comments
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
@ 2025-08-21 10:21 ` Bhupesh
2025-08-21 10:21 ` [PATCH v8 2/5] include: Set tsk->comm length to 64 bytes Bhupesh
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
Patch 3a3f61ce5e0b ("exec: Make sure task->comm is always NUL-terminated"),
replaced 'strscpy_pad()' with 'memcpy()' implementations inside
'__set_task_comm()'.
However a few left-over comments are still there, which mention
the usage of 'strscpy_pad()' inside '__set_task_comm()'.
Remove those obsolete comments.
While at it, also remove an obsolete comment regarding 'task_lock()'
usage while handing 'task->comm'.
Signed-off-by: Bhupesh <bhupesh@igalia.com>
Reviewed-by: Kees Cook <kees@kernel.org>
---
include/linux/sched.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f8188b833350..24216259cda4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1159,10 +1159,8 @@ struct task_struct {
*
* - normally initialized begin_new_exec()
* - set it with set_task_comm()
- * - strscpy_pad() to ensure it is always NUL-terminated and
+ * - logic inside set_task_comm() will ensure it is always NUL-terminated and
* zero-padded
- * - task_lock() to ensure the operation is atomic and the name is
- * fully updated.
*/
char comm[TASK_COMM_LEN];
@@ -1972,7 +1970,7 @@ extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec
* User space can randomly change their names anyway, so locking for readers
* doesn't make sense. For writers, locking is probably necessary, as a race
* condition could lead to long-term mixed results.
- * The strscpy_pad() in __set_task_comm() can ensure that the task comm is
+ * The logic inside __set_task_comm() should ensure that the task comm is
* always NUL-terminated and zero-padded. Therefore the race condition between
* reader and writer is not an issue.
*
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 2/5] include: Set tsk->comm length to 64 bytes
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
2025-08-21 10:21 ` [PATCH v8 1/5] exec: Remove obsolete comments Bhupesh
@ 2025-08-21 10:21 ` Bhupesh
2025-08-21 10:21 ` [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()' Bhupesh
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
Historically due to the 16-byte length of TASK_COMM_LEN, the
users of 'tsk->comm' are restricted to use a fixed-size target
buffer also of TASK_COMM_LEN for 'memcpy()' like use-cases.
To fix the same, we now use a 64-byte TASK_COMM_EXT_LEN and
set the comm element inside 'task_struct' to the same length:
struct task_struct {
.....
char comm[TASK_COMM_EXT_LEN];
.....
};
where TASK_COMM_EXT_LEN is 64-bytes.
Note, that the existing users have not been modified to migrate to
'TASK_COMM_EXT_LEN', in case they have hard-coded expectations of
dealing with only a 'TASK_COMM_LEN' long 'tsk->comm'.
Signed-off-by: Bhupesh <bhupesh@igalia.com>
---
include/linux/sched.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 24216259cda4..bcebc5622e07 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -318,6 +318,7 @@ struct user_event_mm;
*/
enum {
TASK_COMM_LEN = 16,
+ TASK_COMM_EXT_LEN = 64,
};
extern void sched_tick(void);
@@ -1162,7 +1163,7 @@ struct task_struct {
* - logic inside set_task_comm() will ensure it is always NUL-terminated and
* zero-padded
*/
- char comm[TASK_COMM_LEN];
+ char comm[TASK_COMM_EXT_LEN];
struct nameidata *nameidata;
@@ -1961,7 +1962,7 @@ extern void kick_process(struct task_struct *tsk);
extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
#define set_task_comm(tsk, from) ({ \
- BUILD_BUG_ON(sizeof(from) != TASK_COMM_LEN); \
+ BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN); \
__set_task_comm(tsk, from, false); \
})
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
2025-08-21 10:21 ` [PATCH v8 1/5] exec: Remove obsolete comments Bhupesh
2025-08-21 10:21 ` [PATCH v8 2/5] include: Set tsk->comm length to 64 bytes Bhupesh
@ 2025-08-21 10:21 ` Bhupesh
2025-08-22 3:59 ` kernel test robot
2025-08-21 10:21 ` [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
2025-08-21 10:21 ` [PATCH v8 5/5] include: Replace BUILD_BUG_ON with static_assert in 'set_task_comm()' Bhupesh
4 siblings, 1 reply; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
As Linus mentioned in [1], we should get rid of 'get_task_comm()'
entirely and replace it with 'strscpy_pad()' implementation.
'strscpy_pad()' will already make sure comm is NUL-terminated, so
we won't need the explicit final byte termination done in
'get_task_comm()'.
The relevant 'get_task_comm()' users were identified using the
following search pattern:
$ git grep 'get_task_comm*'
Link: https://lore.kernel.org/all/CAHk-=wi5c=_-FBGo_88CowJd_F-Gi6Ud9d=TALm65ReN7YjrMw@mail.gmail.com/ #1
Signed-off-by: Bhupesh <bhupesh@igalia.com>
---
drivers/connector/cn_proc.c | 2 +-
drivers/dma-buf/sw_sync.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c | 2 +-
.../drm/amd/amdgpu/amdgpu_eviction_fence.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 +-
drivers/gpu/drm/lima/lima_ctx.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
drivers/gpu/drm/panthor/panthor_gem.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
drivers/hwtracing/stm/core.c | 2 +-
drivers/tty/tty_audit.c | 2 +-
fs/bcachefs/thread_with_file.c | 2 +-
fs/binfmt_elf.c | 2 +-
fs/binfmt_elf_fdpic.c | 2 +-
fs/ocfs2/cluster/netdebug.c | 1 -
fs/proc/array.c | 2 +-
include/linux/sched.h | 19 -------------------
kernel/audit.c | 6 ++++--
kernel/auditsc.c | 6 ++++--
kernel/sys.c | 2 +-
mm/kmemleak.c | 6 ------
net/bluetooth/hci_sock.c | 2 +-
net/netfilter/nf_tables_api.c | 2 +-
security/integrity/integrity_audit.c | 3 ++-
security/ipe/audit.c | 2 +-
security/landlock/domain.c | 2 +-
security/lsm_audit.c | 7 ++++---
30 files changed, 38 insertions(+), 58 deletions(-)
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 44b19e696176..3c1b07198e10 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -278,7 +278,7 @@ void proc_comm_connector(struct task_struct *task)
ev->what = PROC_EVENT_COMM;
ev->event_data.comm.process_pid = task->pid;
ev->event_data.comm.process_tgid = task->tgid;
- get_task_comm(ev->event_data.comm.comm, task);
+ strscpy_pad(ev->event_data.comm.comm, task->comm);
memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
msg->ack = 0; /* not used */
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 3c20f1d31cf5..1f2ddf00799b 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -311,7 +311,7 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file)
struct sync_timeline *obj;
char task_comm[TASK_COMM_LEN];
- get_task_comm(task_comm, current);
+ strscpy_pad(task_comm, current->comm);
obj = sync_timeline_create(task_comm);
if (!obj)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
index 1ef758ac5076..ea2b99fde425 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
@@ -73,7 +73,7 @@ struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64 context,
/* This reference gets released in amdkfd_fence_release */
mmgrab(mm);
fence->mm = mm;
- get_task_comm(fence->timeline_name, current);
+ strscpy_pad(fence->timeline_name, current->comm);
spin_lock_init(&fence->lock);
fence->svm_bo = svm_bo;
dma_fence_init(&fence->base, &amdkfd_fence_ops, &fence->lock,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index 23d7d0b0d625..3e2ad72cafc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -166,7 +166,7 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
return NULL;
ev_fence->evf_mgr = evf_mgr;
- get_task_comm(ev_fence->timeline_name, current);
+ strscpy_pad(ev_fence->timeline_name, current->comm);
spin_lock_init(&ev_fence->lock);
dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
&ev_fence->lock, evf_mgr->ev_fence_ctx,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 25bade9a5e95..8a4a4c03bbcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -4169,7 +4169,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev)
}
con->init_task_pid = task_pid_nr(current);
- get_task_comm(con->init_task_comm, current);
+ strscpy_pad(con->init_task_comm, current->comm);
mutex_init(&con->critical_region_lock);
INIT_LIST_HEAD(&con->critical_region_head);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index c2a983ff23c9..73aa37f65e0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -92,7 +92,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
fence_drv->adev = adev;
fence_drv->context = dma_fence_context_alloc(1);
- get_task_comm(fence_drv->timeline_name, current);
+ strscpy_pad(fence_drv->timeline_name, current->comm);
xa_lock_irqsave(&adev->userq_xa, flags);
r = xa_err(__xa_store(&adev->userq_xa, userq->doorbell_index,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c3d4bf0767e0..20b83441951b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2523,13 +2523,13 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
return;
vm->task_info->task.pid = current->pid;
- get_task_comm(vm->task_info->task.comm, current);
+ strscpy_pad(vm->task_info->task.comm, current->comm);
if (current->group_leader->mm != current->mm)
return;
vm->task_info->tgid = current->group_leader->pid;
- get_task_comm(vm->task_info->process_name, current->group_leader);
+ strscpy_pad(vm->task_info->process_name, current->group_leader->comm);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index e69db0a93378..7d7c9ff940d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -598,7 +598,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
}
vres->task.pid = task_pid_nr(current);
- get_task_comm(vres->task.comm, current);
+ strscpy_pad(vres->task.comm, current->comm);
list_add_tail(&vres->vres_node, &mgr->allocated_vres_list);
if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS && adjust_dcc_size) {
diff --git a/drivers/gpu/drm/lima/lima_ctx.c b/drivers/gpu/drm/lima/lima_ctx.c
index 0e668fc1e0f9..7536288ec93b 100644
--- a/drivers/gpu/drm/lima/lima_ctx.c
+++ b/drivers/gpu/drm/lima/lima_ctx.c
@@ -29,7 +29,7 @@ int lima_ctx_create(struct lima_device *dev, struct lima_ctx_mgr *mgr, u32 *id)
goto err_out0;
ctx->pid = task_pid_nr(current);
- get_task_comm(ctx->pname, current);
+ strscpy_pad(ctx->pname, current->comm);
return 0;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 85d6289a6eda..75d927020560 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -17,7 +17,7 @@ static void panfrost_gem_debugfs_bo_add(struct panfrost_device *pfdev,
struct panfrost_gem_object *bo)
{
bo->debugfs.creator.tgid = current->group_leader->pid;
- get_task_comm(bo->debugfs.creator.process_name, current->group_leader);
+ strscpy_pad(bo->debugfs.creator.process_name, current->group_leader->comm);
mutex_lock(&pfdev->debugfs.gems_lock);
list_add_tail(&bo->debugfs.node, &pfdev->debugfs.gems_list);
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index a123bc740ba1..ba2b80c760c9 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -27,7 +27,7 @@ static void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo)
struct panthor_device, base);
bo->debugfs.creator.tgid = current->group_leader->pid;
- get_task_comm(bo->debugfs.creator.process_name, current->group_leader);
+ strscpy_pad(bo->debugfs.creator.process_name, current->group_leader->comm);
mutex_lock(&ptdev->gems.lock);
list_add_tail(&bo->debugfs.node, &ptdev->gems.node);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index c33c057365f8..d2bf221e8f01 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -50,7 +50,7 @@ static void virtio_gpu_create_context_locked(struct virtio_gpu_device *vgdev,
} else {
char dbgname[TASK_COMM_LEN];
- get_task_comm(dbgname, current);
+ strscpy_pad(dbgname, current->comm);
virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
vfpriv->context_init, strlen(dbgname),
dbgname);
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index cdba4e875b28..f07784c8751c 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -634,7 +634,7 @@ static ssize_t stm_char_write(struct file *file, const char __user *buf,
char comm[sizeof(current->comm)];
char *ids[] = { comm, "default", NULL };
- get_task_comm(comm, current);
+ strscpy_pad(comm, current->comm);
err = stm_assign_first_policy(stmf->stm, &stmf->output, ids, 1);
/*
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 75542333c54a..2724341edda0 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -77,7 +77,7 @@ static void tty_audit_log(const char *description, dev_t dev,
audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d minor=%d comm=",
description, pid, uid, loginuid, sessionid,
MAJOR(dev), MINOR(dev));
- get_task_comm(name, current);
+ strscpy_pad(name, current->comm);
audit_log_untrustedstring(ab, name);
audit_log_format(ab, " data=");
audit_log_n_hex(ab, data, size);
diff --git a/fs/bcachefs/thread_with_file.c b/fs/bcachefs/thread_with_file.c
index c2eae0ab7765..eda1b6b5e3b7 100644
--- a/fs/bcachefs/thread_with_file.c
+++ b/fs/bcachefs/thread_with_file.c
@@ -35,7 +35,7 @@ int bch2_run_thread_with_file(struct thread_with_file *thr,
fd_flags |= O_WRONLY;
char name[TASK_COMM_LEN];
- get_task_comm(name, current);
+ strscpy_pad(name, current->comm);
thr->ret = 0;
thr->task = kthread_create(fn, thr, "%s", name);
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 264fba0d44bd..6ea946656403 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1540,7 +1540,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_UID(psinfo->pr_uid, from_kuid_munged(cred->user_ns, cred->uid));
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
- get_task_comm(psinfo->pr_fname, p);
+ strscpy_pad(psinfo->pr_fname, p->comm);
return 0;
}
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 48fd2de3bca0..605d09d3c36f 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1365,7 +1365,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_UID(psinfo->pr_uid, from_kuid_munged(cred->user_ns, cred->uid));
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
- get_task_comm(psinfo->pr_fname, p);
+ strscpy_pad(psinfo->pr_fname, p->comm);
return 0;
}
diff --git a/fs/ocfs2/cluster/netdebug.c b/fs/ocfs2/cluster/netdebug.c
index bc27301eab6d..ab94ab55111e 100644
--- a/fs/ocfs2/cluster/netdebug.c
+++ b/fs/ocfs2/cluster/netdebug.c
@@ -122,7 +122,6 @@ static int nst_seq_show(struct seq_file *seq, void *v)
send = ktime_to_us(ktime_sub(now, nst->st_send_time));
status = ktime_to_us(ktime_sub(now, nst->st_status_time));
- /* get_task_comm isn't exported. oh well. */
seq_printf(seq, "%p:\n"
" pid: %lu\n"
" tgid: %lu\n"
diff --git a/fs/proc/array.c b/fs/proc/array.c
index a0a97606c8f6..eefc6b365b3d 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -109,7 +109,7 @@ void proc_task_name(struct seq_file *m, struct task_struct *p, bool escape)
else if (p->flags & PF_KTHREAD)
get_kthread_comm(tcomm, sizeof(tcomm), p);
else
- get_task_comm(tcomm, p);
+ strscpy_pad(tcomm, p->comm);
if (escape)
seq_escape_str(m, tcomm, ESCAPE_SPACE | ESCAPE_SPECIAL, "\n\\");
diff --git a/include/linux/sched.h b/include/linux/sched.h
index bcebc5622e07..5a58c1270474 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1966,25 +1966,6 @@ extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec
__set_task_comm(tsk, from, false); \
})
-/*
- * - Why not use task_lock()?
- * User space can randomly change their names anyway, so locking for readers
- * doesn't make sense. For writers, locking is probably necessary, as a race
- * condition could lead to long-term mixed results.
- * The logic inside __set_task_comm() should ensure that the task comm is
- * always NUL-terminated and zero-padded. Therefore the race condition between
- * reader and writer is not an issue.
- *
- * - BUILD_BUG_ON() can help prevent the buf from being truncated.
- * Since the callers don't perform any return value checks, this safeguard is
- * necessary.
- */
-#define get_task_comm(buf, tsk) ({ \
- BUILD_BUG_ON(sizeof(buf) < TASK_COMM_LEN); \
- strscpy_pad(buf, (tsk)->comm); \
- buf; \
-})
-
static __always_inline void scheduler_ipi(void)
{
/*
diff --git a/kernel/audit.c b/kernel/audit.c
index 61b5744d0bb6..64623f186fc6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1621,7 +1621,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);
audit_log_d_path_exe(ab, current->mm); /* exe= */
audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op, !err);
audit_log_end(ab);
@@ -2270,7 +2271,8 @@ void audit_log_task_info(struct audit_buffer *ab)
audit_get_sessionid(current));
audit_put_tty(tty);
audit_log_format(ab, " comm=");
- audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ strscpy_pad(comm, current->comm);
+ audit_log_untrustedstring(ab, comm);
audit_log_d_path_exe(ab, current->mm);
audit_log_task_context(ab);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 497bda0043fb..801f261d9891 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2931,7 +2931,8 @@ void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
audit_log_format(ab, " pid=%u", task_tgid_nr(current));
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);
audit_log_end(ab);
}
EXPORT_SYMBOL_GPL(__audit_log_nfcfg);
@@ -2954,7 +2955,8 @@ static void audit_log_task(struct audit_buffer *ab)
sessionid);
audit_log_task_context(ab);
audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
- audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ strscpy_pad(comm, current->comm);
+ audit_log_untrustedstring(ab, comm);
audit_log_d_path_exe(ab, current->mm);
}
diff --git a/kernel/sys.c b/kernel/sys.c
index 605f7fe9a143..b139f3fe7539 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2520,7 +2520,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
proc_comm_connector(me);
break;
case PR_GET_NAME:
- get_task_comm(comm, me);
+ strscpy_pad(comm, me->comm);
if (copy_to_user((char __user *)arg2, comm, sizeof(comm)))
return -EFAULT;
break;
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 84265983f239..42e67c03cb7d 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -682,12 +682,6 @@ static struct kmemleak_object *__alloc_object(gfp_t gfp)
strscpy(object->comm, "softirq");
} else {
object->pid = current->pid;
- /*
- * There is a small chance of a race with set_task_comm(),
- * however using get_task_comm() here may cause locking
- * dependency issues with current->alloc_lock. In the worst
- * case, the command line is not correct.
- */
strscpy(object->comm, current->comm);
}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index fc866759910d..759a508024be 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -106,7 +106,7 @@ static bool hci_sock_gen_cookie(struct sock *sk)
id = 0xffffffff;
hci_pi(sk)->cookie = id;
- get_task_comm(hci_pi(sk)->comm, current);
+ strscpy_pad(hci_pi(sk)->comm, current->comm);
return true;
}
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 58c5425d61c2..cdd5195547b5 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9688,7 +9688,7 @@ static int nf_tables_fill_gen_info(struct sk_buff *skb, struct net *net,
if (nla_put_be32(skb, NFTA_GEN_ID, htonl(nft_net->base_seq)) ||
nla_put_be32(skb, NFTA_GEN_PROC_PID, htonl(task_pid_nr(current))) ||
- nla_put_string(skb, NFTA_GEN_PROC_NAME, get_task_comm(buf, current)))
+ nla_put_string(skb, NFTA_GEN_PROC_NAME, strscpy_pad(buf, current->comm)))
goto nla_put_failure;
nlmsg_end(skb, nlh);
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index 0ec5e4c22cb2..28c203f0cdb7 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -54,7 +54,8 @@ void integrity_audit_message(int audit_msgno, struct inode *inode,
audit_get_sessionid(current));
audit_log_task_context(ab);
audit_log_format(ab, " op=%s cause=%s comm=", op, cause);
- audit_log_untrustedstring(ab, get_task_comm(name, current));
+ strscpy_pad(name, current->comm);
+ audit_log_untrustedstring(ab, name);
if (fname) {
audit_log_format(ab, " name=");
audit_log_untrustedstring(ab, fname);
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
index de5fed62592e..b4318988c65d 100644
--- a/security/ipe/audit.c
+++ b/security/ipe/audit.c
@@ -144,7 +144,7 @@ void ipe_audit_match(const struct ipe_eval_ctx *const ctx,
audit_log_format(ab, "ipe_op=%s ipe_hook=%s enforcing=%d pid=%d comm=",
op, audit_hook_names[ctx->hook], READ_ONCE(enforce),
task_tgid_nr(current));
- audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ audit_log_untrustedstring(ab, strscpy_pad(comm, current->comm));
if (ctx->file) {
audit_log_d_path(ab, " path=", &ctx->file->f_path);
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index a647b68e8d06..77b678f0f469 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -102,7 +102,7 @@ static struct landlock_details *get_current_details(void)
memcpy(details->exe_path, path_str, path_size);
details->pid = get_pid(task_tgid(current));
details->uid = from_kuid(&init_user_ns, current_uid());
- get_task_comm(details->comm, current);
+ strscpy_pad(details->comm, current->comm);
return details;
}
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 7d623b00495c..b4d3b8a69cfe 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -276,8 +276,8 @@ void audit_log_lsm_data(struct audit_buffer *ab,
if (pid) {
char tskcomm[sizeof(tsk->comm)];
audit_log_format(ab, " opid=%d ocomm=", pid);
- audit_log_untrustedstring(ab,
- get_task_comm(tskcomm, tsk));
+ strscpy_pad(tskcomm, tsk->comm);
+ audit_log_untrustedstring(ab, tskcomm);
}
}
break;
@@ -417,7 +417,8 @@ static void dump_common_audit_data(struct audit_buffer *ab,
char comm[sizeof(current->comm)];
audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
- audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ strscpy_pad(comm, current->comm);
+ audit_log_untrustedstring(ab, comm);
audit_log_lsm_data(ab, a);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
` (2 preceding siblings ...)
2025-08-21 10:21 ` [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()' Bhupesh
@ 2025-08-21 10:21 ` Bhupesh
2025-08-21 16:43 ` Steven Rostedt
2025-08-25 14:01 ` Kees Cook
2025-08-21 10:21 ` [PATCH v8 5/5] include: Replace BUILD_BUG_ON with static_assert in 'set_task_comm()' Bhupesh
4 siblings, 2 replies; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
As Linus mentioned in [1], currently we have several memcpy() use-cases
which use 'current->comm' to copy the task name over to local copies.
For an example:
...
char comm[TASK_COMM_LEN];
memcpy(comm, current->comm, TASK_COMM_LEN);
...
These should be rather calling a wrappper like "get_task_array()",
which is implemented as:
static __always_inline void
__cstr_array_copy(char *dst,
const char *src, __kernel_size_t size)
{
memcpy(dst, src, size);
dst[size] = 0;
}
#define get_task_array(dst,src) \
__cstr_array_copy(dst, src, __must_be_array(dst))
The relevant 'memcpy()' users were identified using the following search
pattern:
$ git grep 'memcpy.*->comm\>'
Link: https://lore.kernel.org/all/CAHk-=wi5c=_-FBGo_88CowJd_F-Gi6Ud9d=TALm65ReN7YjrMw@mail.gmail.com/ #1
Signed-off-by: Bhupesh <bhupesh@igalia.com>
---
include/linux/coredump.h | 2 +-
include/linux/sched.h | 32 +++++++++++++++++++
include/linux/tracepoint.h | 4 +--
include/trace/events/block.h | 10 +++---
include/trace/events/oom.h | 2 +-
include/trace/events/osnoise.h | 2 +-
include/trace/events/sched.h | 13 ++++----
include/trace/events/signal.h | 2 +-
include/trace/events/task.h | 4 +--
tools/bpf/bpftool/pids.c | 6 ++--
.../bpf/test_kmods/bpf_testmod-events.h | 2 +-
11 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 68861da4cf7c..bcee0afc5eaf 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -54,7 +54,7 @@ extern void vfs_coredump(const kernel_siginfo_t *siginfo);
do { \
char comm[TASK_COMM_LEN]; \
/* This will always be NUL terminated. */ \
- memcpy(comm, current->comm, sizeof(comm)); \
+ get_task_array(comm, current->comm); \
printk_ratelimited(Level "coredump: %d(%*pE): " Format "\n", \
task_tgid_vnr(current), (int)strlen(comm), comm, ##__VA_ARGS__); \
} while (0) \
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 5a58c1270474..d26d1dfb9904 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1960,12 +1960,44 @@ extern void wake_up_new_task(struct task_struct *tsk);
extern void kick_process(struct task_struct *tsk);
+/*
+ * - Why not use task_lock()?
+ * User space can randomly change their names anyway, so locking for readers
+ * doesn't make sense. For writers, locking is probably necessary, as a race
+ * condition could lead to long-term mixed results.
+ * The logic inside __set_task_comm() should ensure that the task comm is
+ * always NUL-terminated and zero-padded. Therefore the race condition between
+ * reader and writer is not an issue.
+ */
+
extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
#define set_task_comm(tsk, from) ({ \
BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN); \
__set_task_comm(tsk, from, false); \
})
+/*
+ * 'get_task_array' can be 'data-racy' in the destination and
+ * should not be used for cases where a 'stable NUL at the end'
+ * is needed. Its better to use strscpy and friends for such
+ * use-cases.
+ *
+ * It is suited mainly for a 'just copy comm to a constant-sized
+ * array' case - especially in performance sensitive use-cases,
+ * like tracing.
+ */
+
+static __always_inline void
+ __cstr_array_copy(char *dst, const char *src,
+ __kernel_size_t size)
+{
+ memcpy(dst, src, size);
+ dst[size] = 0;
+}
+
+#define get_task_array(dst, src) \
+ __cstr_array_copy(dst, src, __must_be_array(dst))
+
static __always_inline void scheduler_ipi(void)
{
/*
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 826ce3f8e1f8..40e04cb660ce 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -570,10 +570,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
* *
*
* TP_fast_assign(
- * memcpy(__entry->next_comm, next->comm, TASK_COMM_LEN);
+ * get_task_array(__entry->next_comm, next->comm);
* __entry->prev_pid = prev->pid;
* __entry->prev_prio = prev->prio;
- * memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN);
+ * get_task_array(__entry->prev_comm, prev->comm);
* __entry->next_pid = next->pid;
* __entry->next_prio = next->prio;
* ),
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 6aa79e2d799c..de1fe35333fc 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -213,7 +213,7 @@ DECLARE_EVENT_CLASS(block_rq,
blk_fill_rwbs(__entry->rwbs, rq->cmd_flags);
__get_str(cmd)[0] = '\0';
- memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, current->comm);
),
TP_printk("%d,%d %s %u (%s) %llu + %u %s,%u,%u [%s]",
@@ -351,7 +351,7 @@ DECLARE_EVENT_CLASS(block_bio,
__entry->sector = bio->bi_iter.bi_sector;
__entry->nr_sector = bio_sectors(bio);
blk_fill_rwbs(__entry->rwbs, bio->bi_opf);
- memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, current->comm);
),
TP_printk("%d,%d %s %llu + %u [%s]",
@@ -434,7 +434,7 @@ TRACE_EVENT(block_plug,
),
TP_fast_assign(
- memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, current->comm);
),
TP_printk("[%s]", __entry->comm)
@@ -453,7 +453,7 @@ DECLARE_EVENT_CLASS(block_unplug,
TP_fast_assign(
__entry->nr_rq = depth;
- memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, current->comm);
),
TP_printk("[%s] %d", __entry->comm, __entry->nr_rq)
@@ -504,7 +504,7 @@ TRACE_EVENT(block_split,
__entry->sector = bio->bi_iter.bi_sector;
__entry->new_sector = new_sector;
blk_fill_rwbs(__entry->rwbs, bio->bi_opf);
- memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, current->comm);
),
TP_printk("%d,%d %s %llu / %llu [%s]",
diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h
index 9f0a5d1482c4..31e5b7295188 100644
--- a/include/trace/events/oom.h
+++ b/include/trace/events/oom.h
@@ -23,7 +23,7 @@ TRACE_EVENT(oom_score_adj_update,
TP_fast_assign(
__entry->pid = task->pid;
- memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, task->comm);
__entry->oom_score_adj = task->signal->oom_score_adj;
),
diff --git a/include/trace/events/osnoise.h b/include/trace/events/osnoise.h
index 3f4273623801..f67f8b5eca75 100644
--- a/include/trace/events/osnoise.h
+++ b/include/trace/events/osnoise.h
@@ -116,7 +116,7 @@ TRACE_EVENT(thread_noise,
),
TP_fast_assign(
- memcpy(__entry->comm, t->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, t->comm);
__entry->pid = t->pid;
__entry->start = start;
__entry->duration = duration;
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 7b2645b50e78..66fe808f2654 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -152,7 +152,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
),
TP_fast_assign(
- memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, p->comm);
__entry->pid = p->pid;
__entry->prio = p->prio; /* XXX SCHED_DEADLINE */
__entry->target_cpu = task_cpu(p);
@@ -237,11 +237,11 @@ TRACE_EVENT(sched_switch,
),
TP_fast_assign(
- memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN);
+ get_task_array(__entry->prev_comm, prev->comm);
__entry->prev_pid = prev->pid;
__entry->prev_prio = prev->prio;
__entry->prev_state = __trace_sched_switch_state(preempt, prev_state, prev);
- memcpy(__entry->next_comm, next->comm, TASK_COMM_LEN);
+ get_task_array(__entry->next_comm, next->comm);
__entry->next_pid = next->pid;
__entry->next_prio = next->prio;
/* XXX SCHED_DEADLINE */
@@ -346,7 +346,7 @@ TRACE_EVENT(sched_process_exit,
),
TP_fast_assign(
- memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, p->comm);
__entry->pid = p->pid;
__entry->prio = p->prio; /* XXX SCHED_DEADLINE */
__entry->group_dead = group_dead;
@@ -787,14 +787,13 @@ TRACE_EVENT(sched_skip_cpuset_numa,
),
TP_fast_assign(
- memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, tsk->comm);
__entry->pid = task_pid_nr(tsk);
__entry->tgid = task_tgid_nr(tsk);
__entry->ngid = task_numa_group_id(tsk);
BUILD_BUG_ON(sizeof(nodemask_t) != \
BITS_TO_LONGS(MAX_NUMNODES) * sizeof(long));
- memcpy(__entry->mem_allowed, mem_allowed_ptr->bits,
- sizeof(__entry->mem_allowed));
+ get_task_array(__entry->mem_allowed, mem_allowed_ptr->bits);
),
TP_printk("comm=%s pid=%d tgid=%d ngid=%d mem_nodes_allowed=%*pbl",
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index 1db7e4b07c01..0681dc5ab1de 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -67,7 +67,7 @@ TRACE_EVENT(signal_generate,
TP_fast_assign(
__entry->sig = sig;
TP_STORE_SIGINFO(__entry, info);
- memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, task->comm);
__entry->pid = task->pid;
__entry->group = group;
__entry->result = result;
diff --git a/include/trace/events/task.h b/include/trace/events/task.h
index af535b053033..9553946943a6 100644
--- a/include/trace/events/task.h
+++ b/include/trace/events/task.h
@@ -21,7 +21,7 @@ TRACE_EVENT(task_newtask,
TP_fast_assign(
__entry->pid = task->pid;
- memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, task->comm);
__entry->clone_flags = clone_flags;
__entry->oom_score_adj = task->signal->oom_score_adj;
),
@@ -44,7 +44,7 @@ TRACE_EVENT(task_rename,
),
TP_fast_assign(
- memcpy(entry->oldcomm, task->comm, TASK_COMM_LEN);
+ get_task_array(entry->oldcomm, task->comm);
strscpy(entry->newcomm, comm, TASK_COMM_LEN);
__entry->oom_score_adj = task->signal->oom_score_adj;
),
diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
index 23f488cf1740..a5d339cb8ca3 100644
--- a/tools/bpf/bpftool/pids.c
+++ b/tools/bpf/bpftool/pids.c
@@ -53,8 +53,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
refs->refs = tmp;
ref = &refs->refs[refs->ref_cnt];
ref->pid = e->pid;
- memcpy(ref->comm, e->comm, sizeof(ref->comm));
- ref->comm[sizeof(ref->comm) - 1] = '\0';
+ get_task_array(ref->comm, e->comm);
refs->ref_cnt++;
return;
@@ -77,8 +76,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
}
ref = &refs->refs[0];
ref->pid = e->pid;
- memcpy(ref->comm, e->comm, sizeof(ref->comm));
- ref->comm[sizeof(ref->comm) - 1] = '\0';
+ get_task_array(ref->comm, e->comm);
refs->ref_cnt = 1;
refs->has_bpf_cookie = e->has_bpf_cookie;
refs->bpf_cookie = e->bpf_cookie;
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
index aeef86b3da74..81880748550f 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
@@ -20,7 +20,7 @@ TRACE_EVENT(bpf_testmod_test_read,
),
TP_fast_assign(
__entry->pid = task->pid;
- memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ get_task_array(__entry->comm, task->comm);
__entry->off = ctx->off;
__entry->len = ctx->len;
),
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 5/5] include: Replace BUILD_BUG_ON with static_assert in 'set_task_comm()'
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
` (3 preceding siblings ...)
2025-08-21 10:21 ` [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
@ 2025-08-21 10:21 ` Bhupesh
4 siblings, 0 replies; 12+ messages in thread
From: Bhupesh @ 2025-08-21 10:21 UTC (permalink / raw)
To: akpm
Cc: bhupesh, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
Replace BUILD_BUG_ON() with static_assert() inside
'set_task_comm()', to benefit from the error message available
with static_assert().
Signed-off-by: Bhupesh <bhupesh@igalia.com>
---
include/linux/sched.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d26d1dfb9904..2603a674ee22 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1972,7 +1972,8 @@ extern void kick_process(struct task_struct *tsk);
extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
#define set_task_comm(tsk, from) ({ \
- BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN); \
+ static_assert(sizeof(from) >= TASK_COMM_LEN, \
+ "tsk->comm size being set should be >= TASK_COMM_LEN"); \
__set_task_comm(tsk, from, false); \
})
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
2025-08-21 10:21 ` [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
@ 2025-08-21 16:43 ` Steven Rostedt
2025-08-22 12:05 ` Bhupesh Sharma
2025-08-25 14:01 ` Kees Cook
1 sibling, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2025-08-21 16:43 UTC (permalink / raw)
To: Bhupesh
Cc: akpm, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
On Thu, 21 Aug 2025 15:51:51 +0530
Bhupesh <bhupesh@igalia.com> wrote:
> +static __always_inline void
> + __cstr_array_copy(char *dst, const char *src,
> + __kernel_size_t size)
> +{
> + memcpy(dst, src, size);
> + dst[size] = 0;
Shouldn't this be: dst[size - 1] = 0;
?
Perhaps also add:
BUILD_BUG_ON(size == 0);
-- Steve
> +}
> +
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
2025-08-21 10:21 ` [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()' Bhupesh
@ 2025-08-22 3:59 ` kernel test robot
2025-08-22 12:06 ` Bhupesh Sharma
0 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2025-08-22 3:59 UTC (permalink / raw)
To: Bhupesh, akpm
Cc: llvm, oe-kbuild-all, bhupesh, kernel-dev, linux-kernel, bpf,
linux-perf-users, linux-fsdevel, linux-mm, oliver.sang, lkp,
laoar.shao, pmladek, rostedt, mathieu.desnoyers, arnaldo.melo,
alexei.starovoitov, andrii.nakryiko, mirq-linux, peterz, willy,
david, viro, keescook, ebiederm, brauner, jack, mingo, juri.lelli,
bsegall, mgorman
Hi Bhupesh,
kernel test robot noticed the following build errors:
[auto build test ERROR on 5303936d609e09665deda94eaedf26a0e5c3a087]
url: https://github.com/intel-lab-lkp/linux/commits/Bhupesh/exec-Remove-obsolete-comments/20250821-182426
base: 5303936d609e09665deda94eaedf26a0e5c3a087
patch link: https://lore.kernel.org/r/20250821102152.323367-4-bhupesh%40igalia.com
patch subject: [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
config: x86_64-buildonly-randconfig-001-20250822 (https://download.01.org/0day-ci/archive/20250822/202508221127.LiaxcbdW-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250822/202508221127.LiaxcbdW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508221127.LiaxcbdW-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/panthor/panthor_sched.c:3420:2: error: call to undeclared function 'get_task_comm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
3420 | get_task_comm(group->task_info.comm, task);
| ^
drivers/gpu/drm/panthor/panthor_sched.c:3420:2: note: did you mean 'get_task_mm'?
include/linux/sched/mm.h:151:26: note: 'get_task_mm' declared here
151 | extern struct mm_struct *get_task_mm(struct task_struct *task);
| ^
1 error generated.
vim +/get_task_comm +3420 drivers/gpu/drm/panthor/panthor_sched.c
de85488138247d Boris Brezillon 2024-02-29 3414
33b9cb6dcda252 Chia-I Wu 2025-07-17 3415 static void group_init_task_info(struct panthor_group *group)
33b9cb6dcda252 Chia-I Wu 2025-07-17 3416 {
33b9cb6dcda252 Chia-I Wu 2025-07-17 3417 struct task_struct *task = current->group_leader;
33b9cb6dcda252 Chia-I Wu 2025-07-17 3418
33b9cb6dcda252 Chia-I Wu 2025-07-17 3419 group->task_info.pid = task->pid;
33b9cb6dcda252 Chia-I Wu 2025-07-17 @3420 get_task_comm(group->task_info.comm, task);
33b9cb6dcda252 Chia-I Wu 2025-07-17 3421 }
33b9cb6dcda252 Chia-I Wu 2025-07-17 3422
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
2025-08-21 16:43 ` Steven Rostedt
@ 2025-08-22 12:05 ` Bhupesh Sharma
0 siblings, 0 replies; 12+ messages in thread
From: Bhupesh Sharma @ 2025-08-22 12:05 UTC (permalink / raw)
To: Steven Rostedt, Bhupesh
Cc: akpm, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, keescook,
ebiederm, brauner, jack, mingo, juri.lelli, bsegall, mgorman,
vschneid, linux-trace-kernel, kees, torvalds
On 8/21/25 10:13 PM, Steven Rostedt wrote:
> On Thu, 21 Aug 2025 15:51:51 +0530
> Bhupesh <bhupesh@igalia.com> wrote:
>
>> +static __always_inline void
>> + __cstr_array_copy(char *dst, const char *src,
>> + __kernel_size_t size)
>> +{
>> + memcpy(dst, src, size);
>> + dst[size] = 0;
> Shouldn't this be: dst[size - 1] = 0;
>
> ?
>
> Perhaps also add:
>
> BUILD_BUG_ON(size == 0);
>
>
Ok, will add this in v9.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
2025-08-22 3:59 ` kernel test robot
@ 2025-08-22 12:06 ` Bhupesh Sharma
0 siblings, 0 replies; 12+ messages in thread
From: Bhupesh Sharma @ 2025-08-22 12:06 UTC (permalink / raw)
To: kernel test robot, Bhupesh, akpm
Cc: llvm, oe-kbuild-all, kernel-dev, linux-kernel, bpf,
linux-perf-users, linux-fsdevel, linux-mm, oliver.sang,
laoar.shao, pmladek, rostedt, mathieu.desnoyers, arnaldo.melo,
alexei.starovoitov, andrii.nakryiko, mirq-linux, peterz, willy,
david, viro, keescook, ebiederm, brauner, jack, mingo, juri.lelli,
bsegall, mgorman
On 8/22/25 9:29 AM, kernel test robot wrote:
> Hi Bhupesh,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 5303936d609e09665deda94eaedf26a0e5c3a087]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Bhupesh/exec-Remove-obsolete-comments/20250821-182426
> base: 5303936d609e09665deda94eaedf26a0e5c3a087
> patch link: https://lore.kernel.org/r/20250821102152.323367-4-bhupesh%40igalia.com
> patch subject: [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()'
> config: x86_64-buildonly-randconfig-001-20250822 (https://download.01.org/0day-ci/archive/20250822/202508221127.LiaxcbdW-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250822/202508221127.LiaxcbdW-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202508221127.LiaxcbdW-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>>> drivers/gpu/drm/panthor/panthor_sched.c:3420:2: error: call to undeclared function 'get_task_comm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> 3420 | get_task_comm(group->task_info.comm, task);
> | ^
> drivers/gpu/drm/panthor/panthor_sched.c:3420:2: note: did you mean 'get_task_mm'?
> include/linux/sched/mm.h:151:26: note: 'get_task_mm' declared here
> 151 | extern struct mm_struct *get_task_mm(struct task_struct *task);
> | ^
> 1 error generated.
>
>
> vim +/get_task_comm +3420 drivers/gpu/drm/panthor/panthor_sched.c
>
> de85488138247d Boris Brezillon 2024-02-29 3414
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3415 static void group_init_task_info(struct panthor_group *group)
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3416 {
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3417 struct task_struct *task = current->group_leader;
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3418
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3419 group->task_info.pid = task->pid;
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 @3420 get_task_comm(group->task_info.comm, task);
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3421 }
> 33b9cb6dcda252 Chia-I Wu 2025-07-17 3422
>
Ok, let me check the same and fix it in v9.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
2025-08-21 10:21 ` [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
2025-08-21 16:43 ` Steven Rostedt
@ 2025-08-25 14:01 ` Kees Cook
2025-09-01 5:28 ` Bhupesh Sharma
1 sibling, 1 reply; 12+ messages in thread
From: Kees Cook @ 2025-08-25 14:01 UTC (permalink / raw)
To: Bhupesh
Cc: akpm, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, ebiederm,
brauner, jack, mingo, juri.lelli, bsegall, mgorman, vschneid,
linux-trace-kernel, torvalds
On Thu, Aug 21, 2025 at 03:51:51PM +0530, Bhupesh wrote:
> As Linus mentioned in [1], currently we have several memcpy() use-cases
> which use 'current->comm' to copy the task name over to local copies.
> For an example:
>
> ...
> char comm[TASK_COMM_LEN];
> memcpy(comm, current->comm, TASK_COMM_LEN);
> ...
>
> These should be rather calling a wrappper like "get_task_array()",
> which is implemented as:
>
> static __always_inline void
> __cstr_array_copy(char *dst,
> const char *src, __kernel_size_t size)
> {
> memcpy(dst, src, size);
> dst[size] = 0;
> }
>
> #define get_task_array(dst,src) \
> __cstr_array_copy(dst, src, __must_be_array(dst))
>
> The relevant 'memcpy()' users were identified using the following search
> pattern:
> $ git grep 'memcpy.*->comm\>'
>
> Link: https://lore.kernel.org/all/CAHk-=wi5c=_-FBGo_88CowJd_F-Gi6Ud9d=TALm65ReN7YjrMw@mail.gmail.com/ #1
>
> Signed-off-by: Bhupesh <bhupesh@igalia.com>
> ---
> include/linux/coredump.h | 2 +-
> include/linux/sched.h | 32 +++++++++++++++++++
> include/linux/tracepoint.h | 4 +--
> include/trace/events/block.h | 10 +++---
> include/trace/events/oom.h | 2 +-
> include/trace/events/osnoise.h | 2 +-
> include/trace/events/sched.h | 13 ++++----
> include/trace/events/signal.h | 2 +-
> include/trace/events/task.h | 4 +--
> tools/bpf/bpftool/pids.c | 6 ++--
> .../bpf/test_kmods/bpf_testmod-events.h | 2 +-
> 11 files changed, 54 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index 68861da4cf7c..bcee0afc5eaf 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -54,7 +54,7 @@ extern void vfs_coredump(const kernel_siginfo_t *siginfo);
> do { \
> char comm[TASK_COMM_LEN]; \
> /* This will always be NUL terminated. */ \
> - memcpy(comm, current->comm, sizeof(comm)); \
> + get_task_array(comm, current->comm); \
> printk_ratelimited(Level "coredump: %d(%*pE): " Format "\n", \
> task_tgid_vnr(current), (int)strlen(comm), comm, ##__VA_ARGS__); \
> } while (0) \
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 5a58c1270474..d26d1dfb9904 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1960,12 +1960,44 @@ extern void wake_up_new_task(struct task_struct *tsk);
>
> extern void kick_process(struct task_struct *tsk);
>
> +/*
> + * - Why not use task_lock()?
> + * User space can randomly change their names anyway, so locking for readers
> + * doesn't make sense. For writers, locking is probably necessary, as a race
> + * condition could lead to long-term mixed results.
> + * The logic inside __set_task_comm() should ensure that the task comm is
> + * always NUL-terminated and zero-padded. Therefore the race condition between
> + * reader and writer is not an issue.
> + */
> +
> extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
> #define set_task_comm(tsk, from) ({ \
> BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN); \
> __set_task_comm(tsk, from, false); \
> })
>
> +/*
> + * 'get_task_array' can be 'data-racy' in the destination and
> + * should not be used for cases where a 'stable NUL at the end'
> + * is needed. Its better to use strscpy and friends for such
> + * use-cases.
> + *
> + * It is suited mainly for a 'just copy comm to a constant-sized
> + * array' case - especially in performance sensitive use-cases,
> + * like tracing.
> + */
> +
> +static __always_inline void
> + __cstr_array_copy(char *dst, const char *src,
> + __kernel_size_t size)
> +{
> + memcpy(dst, src, size);
> + dst[size] = 0;
> +}
Please don't reinvent the wheel. :) We already have memtostr, please use
that (or memtostr_pad).
> +
> +#define get_task_array(dst, src) \
> + __cstr_array_copy(dst, src, __must_be_array(dst))
Uh, __must_be_array(dst) returns 0 on success. :P Are you sure you
tested this?
--
Kees Cook
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
2025-08-25 14:01 ` Kees Cook
@ 2025-09-01 5:28 ` Bhupesh Sharma
0 siblings, 0 replies; 12+ messages in thread
From: Bhupesh Sharma @ 2025-09-01 5:28 UTC (permalink / raw)
To: Kees Cook, Bhupesh
Cc: akpm, kernel-dev, linux-kernel, bpf, linux-perf-users,
linux-fsdevel, linux-mm, oliver.sang, lkp, laoar.shao, pmladek,
rostedt, mathieu.desnoyers, arnaldo.melo, alexei.starovoitov,
andrii.nakryiko, mirq-linux, peterz, willy, david, viro, ebiederm,
brauner, jack, mingo, juri.lelli, bsegall, mgorman, vschneid,
linux-trace-kernel, torvalds
Hi Kees,
On 8/25/25 7:31 PM, Kees Cook wrote:
> On Thu, Aug 21, 2025 at 03:51:51PM +0530, Bhupesh wrote:
>> As Linus mentioned in [1], currently we have several memcpy() use-cases
>> which use 'current->comm' to copy the task name over to local copies.
>> For an example:
>>
>> ...
>> char comm[TASK_COMM_LEN];
>> memcpy(comm, current->comm, TASK_COMM_LEN);
>> ...
>>
>> These should be rather calling a wrappper like "get_task_array()",
>> which is implemented as:
>>
>> static __always_inline void
>> __cstr_array_copy(char *dst,
>> const char *src, __kernel_size_t size)
>> {
>> memcpy(dst, src, size);
>> dst[size] = 0;
>> }
>>
>> #define get_task_array(dst,src) \
>> __cstr_array_copy(dst, src, __must_be_array(dst))
>>
>> The relevant 'memcpy()' users were identified using the following search
>> pattern:
>> $ git grep 'memcpy.*->comm\>'
>>
>> Link:https://lore.kernel.org/all/CAHk-=wi5c=_-FBGo_88CowJd_F-Gi6Ud9d=TALm65ReN7YjrMw@mail.gmail.com/ #1
>>
>> Signed-off-by: Bhupesh<bhupesh@igalia.com>
>> ---
>> include/linux/coredump.h | 2 +-
>> include/linux/sched.h | 32 +++++++++++++++++++
>> include/linux/tracepoint.h | 4 +--
>> include/trace/events/block.h | 10 +++---
>> include/trace/events/oom.h | 2 +-
>> include/trace/events/osnoise.h | 2 +-
>> include/trace/events/sched.h | 13 ++++----
>> include/trace/events/signal.h | 2 +-
>> include/trace/events/task.h | 4 +--
>> tools/bpf/bpftool/pids.c | 6 ++--
>> .../bpf/test_kmods/bpf_testmod-events.h | 2 +-
>> 11 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
>> index 68861da4cf7c..bcee0afc5eaf 100644
>> --- a/include/linux/coredump.h
>> +++ b/include/linux/coredump.h
>> @@ -54,7 +54,7 @@ extern void vfs_coredump(const kernel_siginfo_t *siginfo);
>> do { \
>> char comm[TASK_COMM_LEN]; \
>> /* This will always be NUL terminated. */ \
>> - memcpy(comm, current->comm, sizeof(comm)); \
>> + get_task_array(comm, current->comm); \
>> printk_ratelimited(Level "coredump: %d(%*pE): " Format "\n", \
>> task_tgid_vnr(current), (int)strlen(comm), comm, ##__VA_ARGS__); \
>> } while (0) \
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 5a58c1270474..d26d1dfb9904 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1960,12 +1960,44 @@ extern void wake_up_new_task(struct task_struct *tsk);
>>
>> extern void kick_process(struct task_struct *tsk);
>>
>> +/*
>> + * - Why not use task_lock()?
>> + * User space can randomly change their names anyway, so locking for readers
>> + * doesn't make sense. For writers, locking is probably necessary, as a race
>> + * condition could lead to long-term mixed results.
>> + * The logic inside __set_task_comm() should ensure that the task comm is
>> + * always NUL-terminated and zero-padded. Therefore the race condition between
>> + * reader and writer is not an issue.
>> + */
>> +
>> extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
>> #define set_task_comm(tsk, from) ({ \
>> BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN); \
>> __set_task_comm(tsk, from, false); \
>> })
>>
>> +/*
>> + * 'get_task_array' can be 'data-racy' in the destination and
>> + * should not be used for cases where a 'stable NUL at the end'
>> + * is needed. Its better to use strscpy and friends for such
>> + * use-cases.
>> + *
>> + * It is suited mainly for a 'just copy comm to a constant-sized
>> + * array' case - especially in performance sensitive use-cases,
>> + * like tracing.
>> + */
>> +
>> +static __always_inline void
>> + __cstr_array_copy(char *dst, const char *src,
>> + __kernel_size_t size)
>> +{
>> + memcpy(dst, src, size);
>> + dst[size] = 0;
>> +}
> Please don't reinvent the wheel. :) We already have memtostr, please use
> that (or memtostr_pad).
Sure, but wouldn't we get a static assertion failure: "must be array"
for memtostr() usage, because of the following:
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))
I think it would be easier just to set:
memcpy(dst, src, size);
dst[size -1] = 0;
instead as Linus and Steven suggested.
Thanks,
Bhupesh
>> +
>> +#define get_task_array(dst, src) \
>> + __cstr_array_copy(dst, src, __must_be_array(dst))
> Uh, __must_be_array(dst) returns 0 on success. :P Are you sure you
> tested this?
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-09-01 5:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21 10:21 [PATCH v8 0/5] Add support for long task name Bhupesh
2025-08-21 10:21 ` [PATCH v8 1/5] exec: Remove obsolete comments Bhupesh
2025-08-21 10:21 ` [PATCH v8 2/5] include: Set tsk->comm length to 64 bytes Bhupesh
2025-08-21 10:21 ` [PATCH v8 3/5] treewide: Replace 'get_task_comm()' with 'strscpy_pad()' Bhupesh
2025-08-22 3:59 ` kernel test robot
2025-08-22 12:06 ` Bhupesh Sharma
2025-08-21 10:21 ` [PATCH v8 4/5] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
2025-08-21 16:43 ` Steven Rostedt
2025-08-22 12:05 ` Bhupesh Sharma
2025-08-25 14:01 ` Kees Cook
2025-09-01 5:28 ` Bhupesh Sharma
2025-08-21 10:21 ` [PATCH v8 5/5] include: Replace BUILD_BUG_ON with static_assert in 'set_task_comm()' Bhupesh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).