* [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar]
@ 2021-05-14 6:36 kernel test robot
2021-05-14 14:10 ` Florent Revest
0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2021-05-14 6:36 UTC (permalink / raw)
To: Florent Revest
Cc: kbuild-all, Linux Memory Management List, Alexei Starovoitov
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: ec2618180c3450d06a6a4ba951d4c9a2c689b517
commit: e2d5b2bb769fa5f500760caba76436ba3a10a895 [2296/2770] bpf: Fix nested bpf_bprintf_prepare with more per-cpu buffers
compiler: sparc64-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
>> kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar]
if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) {
^
kernel/bpf/helpers.c:408:20: warning: Possible null pointer dereference: storage [nullPointer]
ptr = &READ_ONCE(storage->buf)->data[0];
^
kernel/bpf/helpers.c:395:39: note: Assignment 'storage=NULL', assigned value is 0
struct bpf_cgroup_storage *storage = NULL;
^
kernel/bpf/helpers.c:408:20: note: Null pointer dereference
ptr = &READ_ONCE(storage->buf)->data[0];
^
vim +713 kernel/bpf/helpers.c
d9c9e4db186ab4 Florent Revest 2021-04-19 705
d9c9e4db186ab4 Florent Revest 2021-04-19 706 static int try_get_fmt_tmp_buf(char **tmp_buf)
d9c9e4db186ab4 Florent Revest 2021-04-19 707 {
e2d5b2bb769fa5 Florent Revest 2021-05-11 708 struct bpf_bprintf_buffers *bufs;
e2d5b2bb769fa5 Florent Revest 2021-05-11 709 int nest_level;
d9c9e4db186ab4 Florent Revest 2021-04-19 710
d9c9e4db186ab4 Florent Revest 2021-04-19 711 preempt_disable();
e2d5b2bb769fa5 Florent Revest 2021-05-11 712 nest_level = this_cpu_inc_return(bpf_bprintf_nest_level);
e2d5b2bb769fa5 Florent Revest 2021-05-11 @713 if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) {
e2d5b2bb769fa5 Florent Revest 2021-05-11 714 this_cpu_dec(bpf_bprintf_nest_level);
d9c9e4db186ab4 Florent Revest 2021-04-19 715 preempt_enable();
d9c9e4db186ab4 Florent Revest 2021-04-19 716 return -EBUSY;
d9c9e4db186ab4 Florent Revest 2021-04-19 717 }
e2d5b2bb769fa5 Florent Revest 2021-05-11 718 bufs = this_cpu_ptr(&bpf_bprintf_bufs);
e2d5b2bb769fa5 Florent Revest 2021-05-11 719 *tmp_buf = bufs->tmp_bufs[nest_level - 1];
d9c9e4db186ab4 Florent Revest 2021-04-19 720
d9c9e4db186ab4 Florent Revest 2021-04-19 721 return 0;
d9c9e4db186ab4 Florent Revest 2021-04-19 722 }
d9c9e4db186ab4 Florent Revest 2021-04-19 723
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] 2021-05-14 6:36 [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] kernel test robot @ 2021-05-14 14:10 ` Florent Revest 2021-05-14 14:17 ` Alexei Starovoitov 0 siblings, 1 reply; 4+ messages in thread From: Florent Revest @ 2021-05-14 14:10 UTC (permalink / raw) To: kernel test robot Cc: kbuild-all, Linux Memory Management List, Alexei Starovoitov On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp@intel.com> wrote: > cppcheck possible warnings: (new ones prefixed by >>, may not real problems) > > >> kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] > if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { > ^ I don't think this is a real problem. bufs is not actually dereferenced, it is only used to give the type information to a sizeof. This is only evaluated at compilation time. If this matters, I guess we could silent this cppcheck warning with something like the following patch. Alexei, what do you think ? --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -697,8 +697,9 @@ static int bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype, #define MAX_PRINTF_BUF_LEN 512 /* Support executing three nested bprintf helper calls on a given CPU */ +#define MAX_PRINTF_NEST_LEVEL 3 struct bpf_bprintf_buffers { - char tmp_bufs[3][MAX_PRINTF_BUF_LEN]; + char tmp_bufs[MAX_PRINTF_NEST_LEVEL][MAX_PRINTF_BUF_LEN]; }; static DEFINE_PER_CPU(struct bpf_bprintf_buffers, bpf_bprintf_bufs); static DEFINE_PER_CPU(int, bpf_bprintf_nest_level); @@ -710,7 +711,7 @@ static int try_get_fmt_tmp_buf(char **tmp_buf) preempt_disable(); nest_level = this_cpu_inc_return(bpf_bprintf_nest_level); - if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { + if (WARN_ON_ONCE(nest_level > MAX_PRINTF_NEST_LEVEL)) { this_cpu_dec(bpf_bprintf_nest_level); preempt_enable(); return -EBUSY; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] 2021-05-14 14:10 ` Florent Revest @ 2021-05-14 14:17 ` Alexei Starovoitov 2021-05-17 9:30 ` Florent Revest 0 siblings, 1 reply; 4+ messages in thread From: Alexei Starovoitov @ 2021-05-14 14:17 UTC (permalink / raw) To: Florent Revest Cc: kernel test robot, kbuild-all, Linux Memory Management List, Alexei Starovoitov On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest@chromium.org> wrote: > > On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp@intel.com> wrote: > > cppcheck possible warnings: (new ones prefixed by >>, may not real problems) > > > > >> kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] > > if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { > > ^ > > I don't think this is a real problem. bufs is not actually > dereferenced, it is only used to give the type information to a > sizeof. This is only evaluated at compilation time. > > If this matters, I guess we could silent this cppcheck warning with > something like the following patch. Alexei, what do you think ? > > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -697,8 +697,9 @@ static int bpf_trace_copy_string(char *buf, void > *unsafe_ptr, char fmt_ptype, > #define MAX_PRINTF_BUF_LEN 512 > > /* Support executing three nested bprintf helper calls on a given CPU */ > +#define MAX_PRINTF_NEST_LEVEL 3 > struct bpf_bprintf_buffers { > - char tmp_bufs[3][MAX_PRINTF_BUF_LEN]; > + char tmp_bufs[MAX_PRINTF_NEST_LEVEL][MAX_PRINTF_BUF_LEN]; > }; > static DEFINE_PER_CPU(struct bpf_bprintf_buffers, bpf_bprintf_bufs); > static DEFINE_PER_CPU(int, bpf_bprintf_nest_level); > @@ -710,7 +711,7 @@ static int try_get_fmt_tmp_buf(char **tmp_buf) > > preempt_disable(); > nest_level = this_cpu_inc_return(bpf_bprintf_nest_level); > - if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { > + if (WARN_ON_ONCE(nest_level > MAX_PRINTF_NEST_LEVEL)) { Yeah. Why not. I think it's cleaner overall. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] 2021-05-14 14:17 ` Alexei Starovoitov @ 2021-05-17 9:30 ` Florent Revest 0 siblings, 0 replies; 4+ messages in thread From: Florent Revest @ 2021-05-17 9:30 UTC (permalink / raw) To: Alexei Starovoitov Cc: kernel test robot, kbuild-all, Linux Memory Management List, Alexei Starovoitov On Fri, May 14, 2021 at 4:18 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest@chromium.org> wrote: > > > > On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp@intel.com> wrote: > > > cppcheck possible warnings: (new ones prefixed by >>, may not real problems) > > > > > > >> kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] > > > if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { > > > ^ > > > > I don't think this is a real problem. bufs is not actually > > dereferenced, it is only used to give the type information to a > > sizeof. This is only evaluated at compilation time. > > > > If this matters, I guess we could silent this cppcheck warning with > > something like the following patch. Alexei, what do you think ? > > > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -697,8 +697,9 @@ static int bpf_trace_copy_string(char *buf, void > > *unsafe_ptr, char fmt_ptype, > > #define MAX_PRINTF_BUF_LEN 512 > > > > /* Support executing three nested bprintf helper calls on a given CPU */ > > +#define MAX_PRINTF_NEST_LEVEL 3 > > struct bpf_bprintf_buffers { > > - char tmp_bufs[3][MAX_PRINTF_BUF_LEN]; > > + char tmp_bufs[MAX_PRINTF_NEST_LEVEL][MAX_PRINTF_BUF_LEN]; > > }; > > static DEFINE_PER_CPU(struct bpf_bprintf_buffers, bpf_bprintf_bufs); > > static DEFINE_PER_CPU(int, bpf_bprintf_nest_level); > > @@ -710,7 +711,7 @@ static int try_get_fmt_tmp_buf(char **tmp_buf) > > > > preempt_disable(); > > nest_level = this_cpu_inc_return(bpf_bprintf_nest_level); > > - if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) { > > + if (WARN_ON_ONCE(nest_level > MAX_PRINTF_NEST_LEVEL)) { > > Yeah. Why not. I think it's cleaner overall. Cool :) thanks, I sent something to the bpf list ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-17 9:30 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-14 6:36 [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar] kernel test robot 2021-05-14 14:10 ` Florent Revest 2021-05-14 14:17 ` Alexei Starovoitov 2021-05-17 9:30 ` Florent Revest
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).