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 | <20260525114241.4b6f3050@pumpkin> |
On Mon, 25 May 2026 11:41:07 +0100 David Laight <[email protected]> wrote: > 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. Or have I got confused over the names... -- David > > > 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: > > >