lkml.org 
[lkml]   [2021]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v5 00/15] extend task comm from 16 to 24 for CONFIG_BASE_FULL
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

\
 
 \ /
  Last update: 2021-10-22 08:22    [W:0.038 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site