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    comm_ext[TASK_COMM_EXT_LEN];
       };

and then modify '__set_task_comm()' to pass 'tsk->comm_ext'
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 "extended 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+cogfd4axrthr2gsi...@mail.gmail.com

Signed-off-by: Bhupesh <bhup...@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 1f5fdd2e096e..3b39fbfc8fe4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1077,11 +1077,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->comm_ext) - 1);
 
        trace_task_rename(tsk, buf);
-       memcpy(tsk->comm, buf, len);
-       memset(&tsk->comm[len], 0, sizeof(tsk->comm) - len);
+       memcpy(tsk->comm_ext, buf, len);
+       memset(&tsk->comm_ext[len], 0, sizeof(tsk->comm_ext) - len);
        perf_event_comm(tsk, exec);
 }
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 704222114dcc..2605207170b4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -316,6 +316,7 @@ struct user_event_mm;
  */
 enum {
        TASK_COMM_LEN = 16,
+       TASK_COMM_EXT_LEN = 64,
 };
 
 extern void sched_tick(void);
@@ -1165,7 +1166,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                    comm_ext[TASK_COMM_EXT_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)->comm_ext);              \
        buf;                                            \
 })
 
-- 
2.38.1


Reply via email to