netdev - Re: [PATCH bpf-next v1 12/19] bpf: add bpf_seq_printf and bpf_seq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 Apr 2020 13:06:02 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     kbuild test robot <lkp@...el.com>,
        Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>, kbuild-all@...ts.01.org,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next v1 12/19] bpf: add bpf_seq_printf and
 bpf_seq_write helpers

On Tue, Apr 28, 2020 at 9:36 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 4/27/20 11:02 PM, kbuild test robot wrote:
> > Hi Yonghong,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on bpf-next/master]
> > [cannot apply to bpf/master net/master vhost/linux-next net-next/master linus/master v5.7-rc3 next-20200424]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://urldefense.proofpoint.com/v2/url?u=https-3A__stackoverflow.com_a_37406982&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=ecuvAWhErc8x32mTscXvNhgSPkwcM7tK05lEVYIQMbI&s=rUkkN8hfXpHttD7t9NCfe5OIFTZZ_cn_SQTDjvs1cj0&e= ]
> >
> > url:    https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-implement-bpf-iterator-for-kernel-data/20200428-115101
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> > config: sh-allmodconfig (attached as .config)
> > compiler: sh4-linux-gcc (GCC) 9.3.0
> > reproduce:
> >          wget https://urldefense.proofpoint.com/v2/url?u=https-3A__raw.githubusercontent.com_intel_lkp-2Dtests_master_sbin_make.cross&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=ecuvAWhErc8x32mTscXvNhgSPkwcM7tK05lEVYIQMbI&s=mm3zd05JFgyD1Fvvg5yehcYq7d9KLZkN7XSYyLaJRkA&e=  -O ~/bin/make.cross
> >          chmod +x ~/bin/make.cross
> >          # save the attached .config to linux build tree
> >          COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=sh
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kbuild test robot <lkp@...el.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> >     In file included from kernel/trace/bpf_trace.c:10:
> >     kernel/trace/bpf_trace.c: In function 'bpf_seq_printf':
> >>> kernel/trace/bpf_trace.c:463:35: warning: the frame size of 1672 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >       463 | BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
>
> Thanks for reporting. Currently, I am supporting up to 12 string format
> specifiers and each string up to 128 bytes. To avoid racing and helper
> memory allocation, I put it on stack hence the above 1672 bytes, but
> practically, I think support 4 strings with 128 bytes each is enough.
> I will make a change in the next revision.

It's still quite a lot of data on stack. How about per-CPU buffer that
this function can use for temporary storage?

>
> >           |                                   ^~~~~~~~
> >     include/linux/filter.h:456:30: note: in definition of macro '__BPF_CAST'
> >       456 |           (unsigned long)0, (t)0))) a
> >           |                              ^
> >>> include/linux/filter.h:449:27: note: in expansion of macro '__BPF_MAP_5'
> >       449 | #define __BPF_MAP(n, ...) __BPF_MAP_##n(__VA_ARGS__)
> >           |                           ^~~~~~~~~~
> >>> include/linux/filter.h:474:35: note: in expansion of macro '__BPF_MAP'
> >       474 |   return ((btf_##name)____##name)(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
> >           |                                   ^~~~~~~~~
> >>> include/linux/filter.h:484:31: note: in expansion of macro 'BPF_CALL_x'
> >       484 | #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
> >           |                               ^~~~~~~~~~
> >>> kernel/trace/bpf_trace.c:463:1: note: in expansion of macro 'BPF_CALL_5'
> >       463 | BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
> >           | ^~~~~~~~~~
> >
> > vim +463 kernel/trace/bpf_trace.c
> >
> >     462
> >   > 463       BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
> >     464                  const void *, data, u32, data_len)
> >     465       {
> >     466               char bufs[MAX_SEQ_PRINTF_VARARGS][MAX_SEQ_PRINTF_STR_LEN];
> >     467               u64 params[MAX_SEQ_PRINTF_VARARGS];
> >     468               int i, copy_size, num_args;
> >     469               const u64 *args = data;
> >     470               int fmt_cnt = 0;
> >     471
> [...]