From: Yafang Shao <laoar.shao@gmail.com>
To: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
Petr Mladek <pmladek@suse.com>,
Peter Zijlstra <peterz@infradead.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Valentin Schneider <valentin.schneider@arm.com>,
Qiang Zhang <qiang.zhang@windriver.com>,
robdclark <robdclark@chromium.org>,
christian <christian@brauner.io>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Ingo Molnar <mingo@redhat.com>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
David Miller <davem@davemloft.net>,
kuba@kernel.org, Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
andrii@kernel.org, kafai@fb.com, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
john.fastabend@gmail.com, kpsingh@kernel.org,
netdev <netdev@vger.kernel.org>,
bpf@vger.kernel.org, linux-perf-users@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
kernel test robot <oliver.sang@intel.com>,
kbuild test robot <lkp@intel.com>
Subject: Re: [PATCH v5 00/15] extend task comm from 16 to 24 for CONFIG_BASE_FULL
Date: Fri, 22 Oct 2021 14:20:55 +0800 [thread overview]
Message-ID: <CALOAHbDFy5sbAaWt=QoVKnq5jborrxEX1Gyzf5+kcomaXtX3rw@mail.gmail.com> (raw)
In-Reply-To: <202110212053.6F3BB603@keescook>
On Fri, Oct 22, 2021 at 12:00 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 21, 2021 at 08:52:22PM -0700, Andrew Morton wrote:
> > On Thu, 21 Oct 2021 03:45:07 +0000 Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > > This patchset changes files among many subsystems. I don't know which
> > > tree it should be applied to, so I just base it on Linus's tree.
> >
> > I can do that ;)
> >
> > > There're many truncated kthreads in the kernel, which may make trouble
> > > for the user, for example, the user can't get detailed device
> > > information from the task comm.
> >
> > That sucked of us.
> >
> > > This patchset tries to improve this problem fundamentally by extending
> > > the task comm size from 16 to 24. In order to do that, we have to do
> > > some cleanups first.
> >
> > It's at v5 and there's no evidence of review activity? C'mon, folks!
>
> It's on my list! :) It's a pretty subtle area that rarely changes, so I
> want to make sure I'm a full coffee to do the review. :)
>
> > > 1. Make the copy of task comm always safe no matter what the task
> > > comm size is. For example,
> > >
> > > Unsafe Safe
> > > strlcpy strscpy_pad
> > > strncpy strscpy_pad
> > > bpf_probe_read_kernel bpf_probe_read_kernel_str
> > > bpf_core_read_str
> > > bpf_get_current_comm
> > > perf_event__prepare_comm
> > > prctl(2)
> > >
> > > 2. Replace the old hard-coded 16 with a new macro TASK_COMM_LEN_16 to
> > > make it more grepable.
> > >
> > > 3. Extend the task comm size to 24 for CONFIG_BASE_FULL case and keep it
> > > as 16 for CONFIG_BASE_SMALL.
> >
> > Is this justified? How much simpler/more reliable/more maintainable/
> > would the code be if we were to make CONFIG_BASE_SMALL suffer with the
> > extra 8 bytes?
>
> Does anyone "own" CONFIG_BASE_SMALL? Gonna go with "no":
>
> $ git ann init/Kconfig| grep 'config BASE_SMALL'
> 1da177e4c3f41 (Linus Torvalds 2005-04-16 15:20:36 -0700 2054)config BASE_SMALL
>
> And it looks mostly unused:
>
> $ git grep CONFIG_BASE_SMALL | cut -d: -f1 | sort -u | xargs -n1 git ann -f | grep 'CONFIG_BASE_SMALL'
> b2af018ff26f1 (Ingo Molnar 2009-01-28 17:36:56 +0100 18)#if CONFIG_BASE_SMALL == 0
> fcdba07ee390d ( Jiri Olsa 2011-02-07 19:31:25 +0100 54)#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
> Blaming lines: 100% (46/46), done.
> 1da177e4c3f41 (Linus Torvalds 2005-04-16 15:20:36 -0700 28)#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000)
> 1da177e4c3f41 (Linus Torvalds 2005-04-16 15:20:36 -0700 34)#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \
> Blaming lines: 100% (162/162), done.
> f86dcc5aa8c79 (Eric Dumazet 2009-10-07 00:37:59 +0000 31)#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256)
> 02c02bf12c5d8 (Matthew Wilcox 2017-11-03 23:09:45 -0400 1110)#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
> a52b89ebb6d44 (Davidlohr Bueso 2014-01-12 15:31:23 -0800 4249)#if CONFIG_BASE_SMALL
> 7b44ab978b77a (Eric W. Biederman 2011-11-16 23:20:58 -0800 78)#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7)
>
>
> --
Right. CONFIG_BASE_SMALL is seldomly used in the kernel.
As you have already removed 64 bytes from task_struct, I think we can
extend the 8 bytes for CONFIG_BASE_SMALL as well.
--
Thanks
Yafang
prev parent reply other threads:[~2021-10-22 6:21 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-21 3:45 [PATCH v5 00/15] extend task comm from 16 to 24 for CONFIG_BASE_FULL Yafang Shao
2021-10-21 3:45 ` [PATCH v5 01/15] fs/exec: make __set_task_comm always set a nul ternimated string Yafang Shao
2021-10-21 3:45 ` [PATCH v5 02/15] fs/exec: make __get_task_comm always get a nul terminated string Yafang Shao
2021-10-21 3:45 ` [PATCH v5 03/15] sched.h: introduce TASK_COMM_LEN_16 Yafang Shao
2021-10-21 21:55 ` Andrii Nakryiko
2021-10-22 6:23 ` Yafang Shao
2021-10-21 3:45 ` [PATCH v5 04/15] cn_proc: make connector comm always nul ternimated Yafang Shao
2021-10-21 3:45 ` [PATCH v5 05/15] drivers/infiniband: make setup_ctxt always get a nul terminated task comm Yafang Shao
2021-10-21 3:45 ` [PATCH v5 06/15] elfcore: make prpsinfo " Yafang Shao
2021-10-21 3:45 ` [PATCH v5 07/15] samples/bpf/kern: use TASK_COMM_LEN instead of hard-coded 16 Yafang Shao
2021-10-21 3:45 ` [PATCH v5 08/15] samples/bpf/user: use TASK_COMM_LEN_16 " Yafang Shao
2021-10-21 3:45 ` [PATCH v5 09/15] tools/include: introduce TASK_COMM_LEN_16 Yafang Shao
2021-10-22 3:52 ` [PATCH v5 00/15] extend task comm from 16 to 24 for CONFIG_BASE_FULL Andrew Morton
2021-10-22 4:00 ` Kees Cook
2021-10-22 6:20 ` Yafang Shao [this message]
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='CALOAHbDFy5sbAaWt=QoVKnq5jborrxEX1Gyzf5+kcomaXtX3rw@mail.gmail.com' \
--to=laoar.shao@gmail.com \
--cc=akpm@linux-foundation.org \
--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=dietmar.eggemann@arm.com \
--cc=john.fastabend@gmail.com \
--cc=juri.lelli@redhat.com \
--cc=kafai@fb.com \
--cc=keescook@chromium.org \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=lkp@intel.com \
--cc=mathieu.desnoyers@efficios.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).