[linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar]
4:36 p.m.
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(a)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
12:10 a.m.
On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp(a)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;
12:17 a.m.
On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest(a)chromium.org> wrote:
On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp(a)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.
7:30 p.m.
On Fri, May 14, 2021 at 4:18 PM Alexei Starovoitov
<alexei.starovoitov(a)gmail.com> wrote:
On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest(a)chromium.org> wrote:
>
> On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp(a)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
491
days inactive
494
days old
3 comments
3 participants
participants (3)
-
Alexei Starovoitov -
Florent Revest -
kernel test robot