From: Kees Cook <keescook@chromium.org>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: akpm@linux-foundation.org, rostedt@goodmis.org,
mathieu.desnoyers@efficios.com, arnaldo.melo@gmail.com,
pmladek@suse.com, peterz@infradead.org, viro@zeniv.linux.org.uk,
valentin.schneider@arm.com, qiang.zhang@windriver.com,
robdclark@chromium.org, christian@brauner.io,
dietmar.eggemann@arm.com, mingo@redhat.com,
juri.lelli@redhat.com, vincent.guittot@linaro.org,
davem@davemloft.net, kuba@kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, kafai@fb.com,
songliubraving@fb.com, yhs@fb.com, john.fastabend@gmail.com,
kpsingh@kernel.org, dennis.dalessandro@cornelisnetworks.com,
mike.marciniszyn@cornelisnetworks.com, dledford@redhat.com,
jgg@ziepe.ca, linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
bpf@vger.kernel.org, linux-perf-users@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, oliver.sang@intel.com,
lkp@intel.com, Andrii Nakryiko <andrii.nakryiko@gmail.com>
Subject: Re: [PATCH v6 05/12] elfcore: make prpsinfo always get a nul terminated task comm
Date: Mon, 25 Oct 2021 14:18:42 -0700 [thread overview]
Message-ID: <202110251417.4D879366@keescook> (raw)
In-Reply-To: <20211025083315.4752-6-laoar.shao@gmail.com>
On Mon, Oct 25, 2021 at 08:33:08AM +0000, Yafang Shao wrote:
> kernel test robot reported a -Wstringop-truncation warning after I
> extend task comm from 16 to 24. Below is the detailed warning:
>
> fs/binfmt_elf.c: In function 'fill_psinfo.isra':
> >> fs/binfmt_elf.c:1575:9: warning: 'strncpy' output may be truncated copying 16 bytes from a string of length 23 [-Wstringop-truncation]
> 1575 | strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This patch can fix this warning.
>
> Replacing strncpy() with strscpy_pad() can avoid this warning.
>
> This patch also replace the hard-coded 16 with TASK_COMM_LEN to make it
> more compatible with task comm size change.
>
> I also verfied if it still work well when I extend the comm size to 24.
> struct elf_prpsinfo is used to dump the task information in userspace
> coredump or kernel vmcore. Below is the verfication of vmcore,
>
> crash> ps
> PID PPID CPU TASK ST %MEM VSZ RSS COMM
> 0 0 0 ffffffff9d21a940 RU 0.0 0 0 [swapper/0]
> > 0 0 1 ffffa09e40f85e80 RU 0.0 0 0 [swapper/1]
> > 0 0 2 ffffa09e40f81f80 RU 0.0 0 0 [swapper/2]
> > 0 0 3 ffffa09e40f83f00 RU 0.0 0 0 [swapper/3]
> > 0 0 4 ffffa09e40f80000 RU 0.0 0 0 [swapper/4]
> > 0 0 5 ffffa09e40f89f80 RU 0.0 0 0 [swapper/5]
> 0 0 6 ffffa09e40f8bf00 RU 0.0 0 0 [swapper/6]
> > 0 0 7 ffffa09e40f88000 RU 0.0 0 0 [swapper/7]
> > 0 0 8 ffffa09e40f8de80 RU 0.0 0 0 [swapper/8]
> > 0 0 9 ffffa09e40f95e80 RU 0.0 0 0 [swapper/9]
> > 0 0 10 ffffa09e40f91f80 RU 0.0 0 0 [swapper/10]
> > 0 0 11 ffffa09e40f93f00 RU 0.0 0 0 [swapper/11]
> > 0 0 12 ffffa09e40f90000 RU 0.0 0 0 [swapper/12]
> > 0 0 13 ffffa09e40f9bf00 RU 0.0 0 0 [swapper/13]
> > 0 0 14 ffffa09e40f98000 RU 0.0 0 0 [swapper/14]
> > 0 0 15 ffffa09e40f9de80 RU 0.0 0 0 [swapper/15]
>
> It works well as expected.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Petr Mladek <pmladek@suse.com>
> ---
> fs/binfmt_elf.c | 2 +-
> include/linux/elfcore-compat.h | 3 ++-
> include/linux/elfcore.h | 4 ++--
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index a813b70f594e..a4ba79fce2a9 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1572,7 +1572,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();
> - strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
> + strscpy_pad(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
This should use get_task_comm().
>
> return 0;
> }
> diff --git a/include/linux/elfcore-compat.h b/include/linux/elfcore-compat.h
> index e272c3d452ce..afa0eb45196b 100644
> --- a/include/linux/elfcore-compat.h
> +++ b/include/linux/elfcore-compat.h
> @@ -5,6 +5,7 @@
> #include <linux/elf.h>
> #include <linux/elfcore.h>
> #include <linux/compat.h>
> +#include <linux/sched.h>
>
> /*
> * Make sure these layouts match the linux/elfcore.h native definitions.
> @@ -43,7 +44,7 @@ struct compat_elf_prpsinfo
> __compat_uid_t pr_uid;
> __compat_gid_t pr_gid;
> compat_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
> - char pr_fname[16];
> + char pr_fname[TASK_COMM_LEN];
> char pr_psargs[ELF_PRARGSZ];
> };
>
> diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
> index 2aaa15779d50..8d79cd58b09a 100644
> --- a/include/linux/elfcore.h
> +++ b/include/linux/elfcore.h
> @@ -65,8 +65,8 @@ struct elf_prpsinfo
> __kernel_gid_t pr_gid;
> pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
> /* Lots missing */
> - char pr_fname[16]; /* filename of executable */
> - char pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
> + char pr_fname[TASK_COMM_LEN]; /* filename of executable */
> + char pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
> };
>
> static inline void elf_core_copy_regs(elf_gregset_t *elfregs, struct pt_regs *regs)
> --
> 2.17.1
>
These structs are externally parsed -- we can't change the size of
pr_fname AFAICT.
--
Kees Cook
next prev parent reply other threads:[~2021-10-25 21:18 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 8:33 [PATCH v6 00/12] extend task comm from 16 to 24 Yafang Shao
2021-10-25 8:33 ` [PATCH v6 01/12] fs/exec: make __set_task_comm always set a nul ternimated string Yafang Shao
2021-10-25 21:07 ` Kees Cook
2021-10-26 1:48 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 02/12] fs/exec: make __get_task_comm always get a nul terminated string Yafang Shao
2021-10-25 21:08 ` Kees Cook
2021-10-26 1:49 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 03/12] drivers/connector: make connector comm always nul ternimated Yafang Shao
2021-10-25 21:14 ` Kees Cook
2021-10-26 1:50 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 04/12] drivers/infiniband: make setup_ctxt always get a nul terminated task comm Yafang Shao
2021-10-25 18:20 ` Dennis Dalessandro
2021-10-25 21:16 ` Kees Cook
2021-10-26 1:52 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 05/12] elfcore: make prpsinfo " Yafang Shao
2021-10-25 21:18 ` Kees Cook [this message]
2021-10-26 1:56 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 06/12] samples/bpf/test_overhead_kprobe_kern: make it adopt to task comm size change Yafang Shao
2021-10-25 21:20 ` Kees Cook
2021-10-25 8:33 ` [PATCH v6 07/12] samples/bpf/offwaketime_kern: make sched_switch tracepoint args adopt to " Yafang Shao
2021-10-25 21:21 ` Kees Cook
2021-10-26 1:56 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 08/12] tools/bpf/bpftool/skeleton: make it adopt to task " Yafang Shao
2021-10-25 21:24 ` Kees Cook
2021-10-26 2:18 ` Yafang Shao
2021-10-26 13:12 ` Steven Rostedt
2021-10-26 13:55 ` Yafang Shao
2021-10-26 14:02 ` Yafang Shao
2021-10-26 16:09 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 09/12] tools/perf/test: make perf test " Yafang Shao
2021-10-25 21:26 ` Kees Cook
2021-10-25 8:33 ` [PATCH v6 10/12] tools/testing/selftests/bpf: make it " Yafang Shao
2021-10-25 21:29 ` Kees Cook
2021-10-26 2:21 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 11/12] sched.h: extend task comm from 16 to 24 Yafang Shao
2021-10-25 21:30 ` Kees Cook
2021-10-26 2:22 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 12/12] kernel/kthread: show a warning if kthread's comm is truncated Yafang Shao
2021-10-25 21:35 ` Kees Cook
2021-10-26 2:23 ` Yafang Shao
2021-10-27 20:10 ` Petr Mladek
2021-10-28 1:42 ` Yafang Shao
2021-10-29 7:44 ` Yafang Shao
2021-10-25 18:10 ` [PATCH v6 00/12] extend task comm from 16 to 24 Alexei Starovoitov
2021-10-25 21:05 ` Steven Rostedt
2021-10-25 21:06 ` Kees Cook
2021-10-26 10:35 ` Petr Mladek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202110251417.4D879366@keescook \
--to=keescook@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=arnaldo.melo@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=christian@brauner.io \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dennis.dalessandro@cornelisnetworks.com \
--cc=dietmar.eggemann@arm.com \
--cc=dledford@redhat.com \
--cc=jgg@ziepe.ca \
--cc=john.fastabend@gmail.com \
--cc=juri.lelli@redhat.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=laoar.shao@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=lkp@intel.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mike.marciniszyn@cornelisnetworks.com \
--cc=mingo@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=oliver.sang@intel.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=qiang.zhang@windriver.com \
--cc=robdclark@chromium.org \
--cc=rostedt@goodmis.org \
--cc=songliubraving@fb.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.guittot@linaro.org \
--cc=viro@zeniv.linux.org.uk \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.