Re: [PATCH v2 4/6] sched: Extend task command name to 64 bytes
David Laight <[email protected]>
| Newsgroups | org.kernel.vger.linux-api,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <20260525114107.7fa5b4c1@pumpkin> |
On Sun, 24 May 2026 19:38:54 -0300 André Almeida <[email protected]> wrote: > Command name has been restrict to only 16 bytes, which is too limiting, > specially when debugging and tracing complex software with thousands of > threads and the need to differentiate them. > > Just as it was done with kthreads in commit 6b59808bfe48 ("workqueue: > Show the latest workqueue name in /proc/PID/{comm,stat,status}"), support > long names for userspace threads as well. > > To avoid buffer overflows, cap all existing userspace APIs to > TASK_COMM_LEN, and leave the full extended name for a new interface. > > Co-developed-by: Bhupesh <[email protected]> > Signed-off-by: Bhupesh <[email protected]> > Signed-off-by: André Almeida <[email protected]> > --- > fs/proc/array.c | 2 +- > include/linux/sched.h | 3 ++- > kernel/sys.c | 10 +++++----- > 3 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/proc/array.c b/fs/proc/array.c > index c8c3fbd9bfa9..312371eddc7f 100644 > --- a/fs/proc/array.c > +++ b/fs/proc/array.c > @@ -110,7 +110,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 > - strscpy_pad(tcomm, p->comm); > + strscpy_pad(tcomm, p->comm, TASK_COMM_LEN); > > 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 b6de742b1155..f7fd2b7d131d 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -323,6 +323,7 @@ struct user_event_mm; > */ > enum { > TASK_COMM_LEN = 16, > + TASK_COMM_EXT_LEN = 64, > }; > > extern void sched_tick(void); > @@ -1167,7 +1168,7 @@ struct task_struct { > * - set it with set_task_comm() to ensure it is always > * NUL-terminated and zero-padded > */ > - char comm[TASK_COMM_LEN]; > + char comm[TASK_COMM_EXT_LEN]; > > struct nameidata *nameidata; > > diff --git a/kernel/sys.c b/kernel/sys.c > index 1d5152d2395e..76d77218ab19 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -2535,7 +2535,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > unsigned long, arg4, unsigned long, arg5) > { > struct task_struct *me = current; > - unsigned char comm[sizeof(me->comm)]; > + unsigned char comm[TASK_COMM_LEN]; > long error; > > error = security_task_prctl(option, arg2, arg3, arg4, arg5); > @@ -2601,16 +2601,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > error = -EINVAL; > break; > case PR_SET_NAME: > - comm[sizeof(me->comm) - 1] = 0; > + comm[TASK_COMM_LEN - 1] = 0; > if (strncpy_from_user(comm, (char __user *)arg2, > - sizeof(me->comm) - 1) < 0) > + TASK_COMM_LEN - 1) < 0) Nak - you can't do that. You are reading data that the application doesn't expect you to read. > return -EFAULT; > set_task_comm(me, comm); > proc_comm_connector(me); > break; > case PR_GET_NAME: > - strscpy_pad(comm, me->comm); > - if (copy_to_user((char __user *)arg2, comm, sizeof(comm))) > + strscpy_pad(comm, me->comm, TASK_COMM_LEN); > + if (copy_to_user((char __user *)arg2, comm, TASK_COMM_LEN)) Double-nak - you are writing beyond the end of the applications buffer. You can't change the user memory that the syscalls access. You can support the longer name for read/write of /proc/self/comm. -- David > return -EFAULT; > break; > case PR_GET_ENDIAN: >