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, Linus suggested in [1] that we can add the
following union inside 'task_struct':
       union {
               char    comm[TASK_COMM_LEN];
               char    real_comm[REAL_TASK_COMM_LEN];
       };

and then modify '__set_task_comm()' to pass 'tsk->real_comm'
to the existing users.

This would mean that:
(1) The old common pattern of just printing with '%s' and tsk->comm
    would just continue to work (as it is):

        pr_alert("BUG: Bad page state in process %s  pfn:%05lx\n",
                current->comm, page_to_pfn(page));

(2) And, the memcpy() users of 'tsk->comm' would need to be made more
    stable by ensuring that the destination buffer always has a closing
    NUL character (done already in the preceding patch in this series).

So, eventually:
- users who want the existing 'TASK_COMM_LEN' behavior will get it
  (existing ABIs would continue to work),
- users who just print out 'tsk->comm' as a string will get the longer
  new "real comm",
- users who do 'sizeof(->comm)' will continue to get the old value
  because of the union.

[1]. https://lore.kernel.org/all/CAHk-=wjAmmHUg6vho1KjzQi2=psR30+CogFd4aXrThr2gsiS4g@mail.gmail.com

Signed-off-by: Bhupesh <bhupesh@igalia.com>
---
 fs/exec.c             | 6 +++---
 include/linux/sched.h | 8 ++++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8e4ea5f1e64c..2b2f2dacc013 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1190,11 +1190,11 @@ static int unshare_sighand(struct task_struct *me)
  */
 void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec)
 {
-	size_t len = min(strlen(buf), sizeof(tsk->comm) - 1);
+	size_t len = min(strlen(buf), sizeof(tsk->real_comm) - 1);
 
 	trace_task_rename(tsk, buf);
-	memcpy(tsk->comm, buf, len);
-	memset(&tsk->comm[len], 0, sizeof(tsk->comm) - len);
+	memcpy(tsk->real_comm, buf, len);
+	memset(&tsk->real_comm[len], 0, sizeof(tsk->real_comm) - len);
 	perf_event_comm(tsk, exec);
 }
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index cb219c6db179..2744d90badf1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -317,6 +317,7 @@ struct user_event_mm;
  */
 enum {
 	TASK_COMM_LEN = 16,
+	REAL_TASK_COMM_LEN = 64,
 };
 
 extern void sched_tick(void);
@@ -1162,7 +1163,10 @@ struct task_struct {
 	 *   - logic inside set_task_comm() will ensure it is always NUL-terminated and
 	 *     zero-padded
 	 */
-	char				comm[TASK_COMM_LEN];
+	union {
+		char			comm[TASK_COMM_LEN];
+		char			real_comm[REAL_TASK_COMM_LEN];
+	};
 
 	struct nameidata		*nameidata;
 
@@ -2005,7 +2009,7 @@ extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec
  */
 #define get_task_comm(buf, tsk) ({			\
 	BUILD_BUG_ON(sizeof(buf) < TASK_COMM_LEN);	\
-	strscpy_pad(buf, (tsk)->comm);			\
+	strscpy_pad(buf, (tsk)->real_comm);		\
 	buf;						\
 })
 
-- 
2.38.1

On Wed 2025-05-07 16:34:44, Bhupesh wrote:
> 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, Linus suggested in [1] that we can add the
> following union inside 'task_struct':
>        union {
>                char    comm[TASK_COMM_LEN];
>                char    real_comm[REAL_TASK_COMM_LEN];
>        };

Nit: IMHO, the prefix "real_" is misleading. The buffer size is still
      limited and the name might be shrinked. I would suggest
      something like:

	char    comm_ext[TASK_COMM_EXT_LEN];
or
	char    comm_64[TASK_COMM_64_LEN]

> and then modify '__set_task_comm()' to pass 'tsk->real_comm'
> to the existing users.

Best Regards,
Petr

On Wed, 7 May 2025 14:54:36 +0200
Petr Mladek <pmladek@suse.com> wrote:

> > To fix the same, Linus suggested in [1] that we can add the
> > following union inside 'task_struct':
> >        union {
> >                char    comm[TASK_COMM_LEN];
> >                char    real_comm[REAL_TASK_COMM_LEN];
> >        };  
> 
> Nit: IMHO, the prefix "real_" is misleading. The buffer size is still
>       limited and the name might be shrinked. I would suggest

Agreed.

>       something like:
> 
> 	char    comm_ext[TASK_COMM_EXT_LEN];
> or
> 	char    comm_64[TASK_COMM_64_LEN]

I prefer "comm_ext" as I don't think we want to hard code the actual size.
Who knows, in the future we may extend it again!

-- Steve

On 5/7/25 10:53 PM, Steven Rostedt wrote:
> On Wed, 7 May 2025 14:54:36 +0200
> Petr Mladek <pmladek@suse.com> wrote:
>
>>> To fix the same, Linus suggested in [1] that we can add the
>>> following union inside 'task_struct':
>>>         union {
>>>                 char    comm[TASK_COMM_LEN];
>>>                 char    real_comm[REAL_TASK_COMM_LEN];
>>>         };
>> Nit: IMHO, the prefix "real_" is misleading. The buffer size is still
>>        limited and the name might be shrinked. I would suggest
> Agreed.
>
>>        something like:
>>
>> 	char    comm_ext[TASK_COMM_EXT_LEN];
>> or
>> 	char    comm_64[TASK_COMM_64_LEN]
> I prefer "comm_ext" as I don't think we want to hard code the actual size.
> Who knows, in the future we may extend it again!
>

Ok, let me use 'comm_64' instead in v4.

Thanks for the review.

Regards,
Bhupesh

On Thu, 8 May 2025 13:38:11 +0530
Bhupesh Sharma <bhsharma@igalia.com> wrote:

> >>        something like:
> >>
> >> 	char    comm_ext[TASK_COMM_EXT_LEN];
> >> or
> >> 	char    comm_64[TASK_COMM_64_LEN]  
> > I prefer "comm_ext" as I don't think we want to hard code the actual size.
> > Who knows, in the future we may extend it again!
> >  
> 
> Ok, let me use 'comm_64' instead in v4.

I think you missed what I said. I prefer the comm_ext over comm_64 because
I don't think it's a good idea to hardcode the extended size in the name.
That will make it very difficult in the future if we want to make it even
larger.

-- Steve