On Thu, Oct 21, 2021 at 08:52:22PM -0700, Andrew Morton wrote:

On Thu, 21 Oct 2021 03:45:07 +0000 Yafang Shao [off-list ref] wrote:

quoted

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 ;)

quoted

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.

quoted

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. :)

quoted

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)


-- 
Kees Cook