* [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).