bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).