From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1B9DBC433F5 for ; Tue, 26 Oct 2021 01:56:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9943160FC2 for ; Tue, 26 Oct 2021 01:56:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9943160FC2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 26551940008; Mon, 25 Oct 2021 21:56:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2162B940007; Mon, 25 Oct 2021 21:56:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DD99940008; Mon, 25 Oct 2021 21:56:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0186.hostedemail.com [216.40.44.186]) by kanga.kvack.org (Postfix) with ESMTP id ED0FE940007 for ; Mon, 25 Oct 2021 21:56:52 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id AAA35183B04AC for ; Tue, 26 Oct 2021 01:56:52 +0000 (UTC) X-FDA: 78736925064.26.57C49BE Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by imf08.hostedemail.com (Postfix) with ESMTP id C4BE330000AE for ; Tue, 26 Oct 2021 01:56:45 +0000 (UTC) Received: by mail-io1-f42.google.com with SMTP id y67so18111336iof.10 for ; Mon, 25 Oct 2021 18:56:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xWeUj1T4Lr60JoRsLSjKYzLtDKrcRSE1xJJaTRkfYu0=; b=gAapokdrHWuAyvsseWO0TvIdywOQUK0S407ePIzmBXjmKHIKH7NnSGxsvX9iKiES2N HLwuffairU//rIaPVQ7ez8hSwM91YyJ7d3VUtGe56ciYfAVTOhAUPEMmlZNxhUjRHPeP BtXSS/9svwqyTRQ+lHY3Ka1U0E7iVJfsIN6OD1hucX8L4TePBv4zmsOosvIRNQDTtPCk jTlTc8jliDg0tD/CmcRWdUr2YtK7LHcDeitU4M/+Fc+8DgtMfBM+h+e4Qz0XJ+H3wBFu rWVej7/USxWcq4JINLUJ0HukPwZI5emI+RM9PYxx6Icx++KKVF0uuHn9dAW+oFohue2K DJ+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xWeUj1T4Lr60JoRsLSjKYzLtDKrcRSE1xJJaTRkfYu0=; b=QFDt0QsIS3xpEM9p7pk3YbAcEPy6Pl/DlqwsXZs7PmTo3sJJX5JgfulHGJd97w46F7 kj1MD70l2R1DIrGe/VGdaCbwI8cpbPCr+HnDyiu4jGXv2Lvov5cAeGhSB28ypzcKr6bZ dXaAhH9u51An2F+f6b6EzHul2BRYn1wNvbHtKXUYOawNK9LgpOEGsm3kZdygj33++FOj Pma++h5swNJu4uT+ojZ0w8+RuXIw4OGVVDYZreH9NLVJM1guMnqJJkO7qJSVyIbHVAsz R72q7ZrB1d/lKT1LSAV889GRsYI+2wt5PjsrXCghXSj+WupBtuRRwe3ymQIqORB1VKup WT7A== X-Gm-Message-State: AOAM533kZAzXhnc8BGLpcikyhgIEr087HnEPx+koNocvdcA1q37tsToI 5O+o+3NakTMsV1XmIPoaxLuIYvQca7Cj3ILZceQ= X-Google-Smtp-Source: ABdhPJzn3jxml3ZFy8UfHJXcY4mdjkvn7tNH2JicBIze1RxSUpk5VNwer/ZyY/WbY+4kBOP+NVYRRevs/EoFq1bKkdE= X-Received: by 2002:a02:aa96:: with SMTP id u22mr13403998jai.95.1635213411760; Mon, 25 Oct 2021 18:56:51 -0700 (PDT) MIME-Version: 1.0 References: <20211025083315.4752-1-laoar.shao@gmail.com> <20211025083315.4752-6-laoar.shao@gmail.com> <202110251417.4D879366@keescook> In-Reply-To: <202110251417.4D879366@keescook> From: Yafang Shao Date: Tue, 26 Oct 2021 09:56:15 +0800 Message-ID: Subject: Re: [PATCH v6 05/12] elfcore: make prpsinfo always get a nul terminated task comm To: Kees Cook Cc: Andrew Morton , Steven Rostedt , Mathieu Desnoyers , Arnaldo Carvalho de Melo , Petr Mladek , Peter Zijlstra , Al Viro , Valentin Schneider , Qiang Zhang , robdclark , christian , Dietmar Eggemann , Ingo Molnar , Juri Lelli , Vincent Guittot , David Miller , Jakub Kicinski , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin Lau , Song Liu , Yonghong Song , john fastabend , KP Singh , dennis.dalessandro@cornelisnetworks.com, mike.marciniszyn@cornelisnetworks.com, dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, netdev , bpf , "linux-perf-use." , linux-fsdevel@vger.kernel.org, Linux MM , LKML , kernel test robot , kbuild test robot , Andrii Nakryiko Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: C4BE330000AE X-Stat-Signature: kfdhjwf9bfcta5g1o9i6jg4xkirotsac Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=gAapokdr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.166.42 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com X-HE-Tag: 1635213405-779735 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Oct 26, 2021 at 5:18 AM Kees Cook wrote: > > 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 > > Signed-off-by: Yafang Shao > > Cc: Mathieu Desnoyers > > Cc: Arnaldo Carvalho de Melo > > Cc: Andrii Nakryiko > > Cc: Peter Zijlstra > > Cc: Steven Rostedt > > Cc: Al Viro > > Cc: Kees Cook > > Cc: Petr Mladek > > --- > > 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(). > Sure. > > > > 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 > > #include > > #include > > +#include > > > > /* > > * 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. > Yes, they are parsed by crash utility and other tools. I will keep pr_fname as-is. -- Thanks Yafang