* [PATCH bpf-next 0/2] bpf: fix NULL pointer dereference in
@ 2021-03-19 0:17 Yonghong Song
2021-03-19 0:18 ` [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper Yonghong Song
2021-03-19 0:18 ` [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run Yonghong Song
0 siblings, 2 replies; 6+ messages in thread
From: Yonghong Song @ 2021-03-19 0:17 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Jiri Olsa reported a bug in
https://lore.kernel.org/bpf/CAKH8qBuXCfUz=w8L+Fj74OaUpbosO29niYwTki7e3Ag044_aww@mail.gmail.com/T
where cgroup local storage pointer may be NULL in bpf_get_local_storage()
helper. There are two issues uncovered by this bug:
(1). kprobe or tracepoint prog incorrectly sets cgroup local storage
before prog run,
(2). due to change from preempt_disable to migrate_disable,
preemption is possible and percpu storage might be overwritten
by other tasks.
Issue (1) has been fixed and this patch set fixed issue (2).
Please see details of Patch #1 for detailed fix.
Patch #2 fixed bpf_prog_test_run to use new signature for
bpf_cgroup_storage_set().
Patch #1 can be backported to bpf tree and Patch #2 cannot due to
refactoring done in bpf-next only.
I did not put a Fix tag as I am not able to find a proper one.
The original commit which changed from preempt_disable to
migrate_disable in bpf.h is just a wrapper where migrate_disable
still calls preempt_disable. The real migrate_disable change happens
later in kernel/sched/*.
Yonghong Song (2):
bpf: fix NULL pointer dereference in bpf_get_local_storage() helper
bpf: fix bpf_cgroup_storage_set() usage in test_run
include/linux/bpf-cgroup.h | 53 +++++++++++++++++++++++++++++++++-----
include/linux/bpf.h | 22 +++++++++++++---
kernel/bpf/helpers.c | 15 ++++++++---
kernel/bpf/local_storage.c | 3 ++-
net/bpf/test_run.c | 6 ++++-
5 files changed, 83 insertions(+), 16 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper 2021-03-19 0:17 [PATCH bpf-next 0/2] bpf: fix NULL pointer dereference in Yonghong Song @ 2021-03-19 0:18 ` Yonghong Song 2021-03-19 1:37 ` kernel test robot 2021-03-19 2:40 ` kernel test robot 2021-03-19 0:18 ` [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run Yonghong Song 1 sibling, 2 replies; 6+ messages in thread From: Yonghong Song @ 2021-03-19 0:18 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Jiri Olsa, Roman Gushchin Jiri Olsa reported a bug ([1]) in kernel where cgroup local storage pointer may be NULL in bpf_get_local_storage() helper. There are two issues uncovered by this bug: (1). kprobe or tracepoint prog incorrectly sets cgroup local storage before prog run, (2). due to change from preempt_disable to migrate_disable, preemption is possible and percpu storage might be overwritten by other tasks. This issue (1) is fixed in [2]. This patch tried to address issue (2). The following shows how things can go wrong: task 1: bpf_cgroup_storage_set() for percpu local storage preemption happens task 2: bpf_cgroup_storage_set() for percpu local storage preemption happens task 1: run bpf program task 1 will effectively use the percpu local storage setting by task 2 which will be either NULL or incorrect ones. Instead of just one common local storage per cpu, this patch fixed the issue by permitting 8 local storages per cpu and each local storage is identified by a task_struct pointer. This way, we allow at most 8 nested preemption between bpf_cgroup_storage_set() and bpf_cgroup_storage_unset(). The percpu local storage slot is released (calling bpf_cgroup_storage_unset()) by the same task after bpf program finished running. The patch is tested on top of [2] with reproducer in [1]. Without this patch, kernel will emit error in 2-3 minutes. With this patch, after one hour, still no error. [1] https://lore.kernel.org/bpf/CAKH8qBuXCfUz=w8L+Fj74OaUpbosO29niYwTki7e3Ag044_aww@mail.gmail.com/T [2] https://lore.kernel.org/bpf/CAKH8qBuXCfUz=w8L+Fj74OaUpbosO29niYwTki7e3Ag044_aww@mail.gmail.com/T Cc: Jiri Olsa <jolsa@kernel.org> Cc: Roman Gushchin <guro@fb.com> Signed-off-by: Yonghong Song <yhs@fb.com> --- include/linux/bpf-cgroup.h | 53 +++++++++++++++++++++++++++++++++----- include/linux/bpf.h | 22 +++++++++++++--- kernel/bpf/helpers.c | 15 ++++++++--- kernel/bpf/local_storage.c | 3 ++- 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h index c42e02b4d84b..96fe26509489 100644 --- a/include/linux/bpf-cgroup.h +++ b/include/linux/bpf-cgroup.h @@ -20,14 +20,25 @@ struct bpf_sock_ops_kern; struct bpf_cgroup_storage; struct ctl_table; struct ctl_table_header; +struct task_struct; #ifdef CONFIG_CGROUP_BPF extern struct static_key_false cgroup_bpf_enabled_key[MAX_BPF_ATTACH_TYPE]; #define cgroup_bpf_enabled(type) static_branch_unlikely(&cgroup_bpf_enabled_key[type]) -DECLARE_PER_CPU(struct bpf_cgroup_storage*, - bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]); +#define BPF_CGROUP_STORAGE_NEST_MAX 8 + +struct bpf_cgroup_storage_info { + struct task_struct *task; + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]; +}; + +/* For each cpu, permit maximum BPF_CGROUP_STORAGE_NEST_MAX number of tasks + * to use bpf cgroup storage simultaneously. + */ +DECLARE_PER_CPU(struct bpf_cgroup_storage_info, + bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); #define for_each_cgroup_storage_type(stype) \ for (stype = 0; stype < MAX_BPF_CGROUP_STORAGE_TYPE; stype++) @@ -161,13 +172,43 @@ static inline enum bpf_cgroup_storage_type cgroup_storage_type( return BPF_CGROUP_STORAGE_SHARED; } -static inline void bpf_cgroup_storage_set(struct bpf_cgroup_storage - *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) +static inline int bpf_cgroup_storage_set(struct bpf_cgroup_storage + *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) { enum bpf_cgroup_storage_type stype; + int i; + + preempt_disable(); + for (i = 0; i < BPF_CGROUP_STORAGE_NEST_MAX; i++) { + if (unlikely(this_cpu_read(bpf_cgroup_storage_info[i].task) != NULL)) + continue; + + this_cpu_write(bpf_cgroup_storage_info[i].task, current); + for_each_cgroup_storage_type(stype) + this_cpu_write(bpf_cgroup_storage_info[i].storage[stype], + storage[stype]); + break; + } + preempt_enable(); + + if (i == BPF_CGROUP_STORAGE_NEST_MAX) { + WARN_ON_ONCE(1); + return -EBUSY; + } + return 0; +} + +static inline void bpf_cgroup_storage_unset(void) +{ + int i; + + for (i = 0; i < BPF_CGROUP_STORAGE_NEST_MAX; i++) { + if (unlikely(this_cpu_read(bpf_cgroup_storage_info[i].task) != current)) + continue; - for_each_cgroup_storage_type(stype) - this_cpu_write(bpf_cgroup_storage[stype], storage[stype]); + this_cpu_write(bpf_cgroup_storage_info[i].task, NULL); + return; + } } struct bpf_cgroup_storage * diff --git a/include/linux/bpf.h b/include/linux/bpf.h index a47285cd39c2..3a6ae69743ff 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1090,6 +1090,13 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array, /* BPF program asks to set CN on the packet. */ #define BPF_RET_SET_CN (1 << 0) +/* For BPF_PROG_RUN_ARRAY_FLAGS and __BPF_PROG_RUN_ARRAY, + * if bpf_cgroup_storage_set() failed, the rest of programs + * will not execute. This should be a really rare scenario + * as it requires BPF_CGROUP_STORAGE_NEST_MAX number of + * preemptions all between bpf_cgroup_storage_set() and + * bpf_cgroup_storage_unset() on the same cpu. + */ #define BPF_PROG_RUN_ARRAY_FLAGS(array, ctx, func, ret_flags) \ ({ \ struct bpf_prog_array_item *_item; \ @@ -1102,10 +1109,12 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array, _array = rcu_dereference(array); \ _item = &_array->items[0]; \ while ((_prog = READ_ONCE(_item->prog))) { \ - bpf_cgroup_storage_set(_item->cgroup_storage); \ + if (unlikely(bpf_cgroup_storage_set(_item->cgroup_storage))) \ + break; \ func_ret = func(_prog, ctx); \ _ret &= (func_ret & 1); \ *(ret_flags) |= (func_ret >> 1); \ + bpf_cgroup_storage_unset(); \ _item++; \ } \ rcu_read_unlock(); \ @@ -1126,9 +1135,14 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array, goto _out; \ _item = &_array->items[0]; \ while ((_prog = READ_ONCE(_item->prog))) { \ - if (set_cg_storage) \ - bpf_cgroup_storage_set(_item->cgroup_storage); \ - _ret &= func(_prog, ctx); \ + if (!set_cg_storage) { \ + _ret &= func(_prog, ctx); \ + } else { \ + if (unlikely(bpf_cgroup_storage_set(_item->cgroup_storage))) \ + break; \ + _ret &= func(_prog, ctx); \ + bpf_cgroup_storage_unset(); \ + } \ _item++; \ } \ _out: \ diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 074800226327..f306611c4ddf 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -382,8 +382,8 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = { }; #ifdef CONFIG_CGROUP_BPF -DECLARE_PER_CPU(struct bpf_cgroup_storage*, - bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]); +DECLARE_PER_CPU(struct bpf_cgroup_storage_info, + bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags) { @@ -392,10 +392,17 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags) * verifier checks that its value is correct. */ enum bpf_cgroup_storage_type stype = cgroup_storage_type(map); - struct bpf_cgroup_storage *storage; + struct bpf_cgroup_storage *storage = NULL; void *ptr; + int i; - storage = this_cpu_read(bpf_cgroup_storage[stype]); + for (i = 0; i < BPF_CGROUP_STORAGE_NEST_MAX; i++) { + if (unlikely(this_cpu_read(bpf_cgroup_storage_info[i].task) != current)) + continue; + + storage = this_cpu_read(bpf_cgroup_storage_info[i].storage[stype]); + break; + } if (stype == BPF_CGROUP_STORAGE_SHARED) ptr = &READ_ONCE(storage->buf)->data[0]; diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c index 2d4f9ac12377..174350524577 100644 --- a/kernel/bpf/local_storage.c +++ b/kernel/bpf/local_storage.c @@ -9,7 +9,8 @@ #include <linux/slab.h> #include <uapi/linux/btf.h> -DEFINE_PER_CPU(struct bpf_cgroup_storage*, bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]); +DEFINE_PER_CPU(struct bpf_cgroup_storage_info, + bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); #ifdef CONFIG_CGROUP_BPF -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper 2021-03-19 0:18 ` [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper Yonghong Song @ 2021-03-19 1:37 ` kernel test robot 2021-03-19 2:40 ` kernel test robot 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2021-03-19 1:37 UTC (permalink / raw) To: Yonghong Song, bpf Cc: kbuild-all, clang-built-linux, Alexei Starovoitov, Daniel Borkmann, kernel-team, Jiri Olsa, Roman Gushchin [-- Attachment #1: Type: text/plain, Size: 16471 bytes --] Hi Yonghong, I love your patch! Yet something to improve: [auto build test ERROR on next-20210318] [cannot apply to bpf-next/master bpf/master v5.12-rc3 v5.12-rc2 v5.12-rc1 v5.12-rc3] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 base: ba5b053ab3ac674b91a6669086139819359a5e6e config: powerpc-randconfig-r034-20210318 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc cross compiling tool for clang build # apt-get install binutils-powerpc-linux-gnu # https://github.com/0day-ci/linux/commit/38b56e03d083be940fe9dc231c5f6c8f4f282f15 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 git checkout 38b56e03d083be940fe9dc231c5f6c8f4f282f15 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:210:1: note: expanded from here __do_insb ^ arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb' #define __do_insb(p, b, n) readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/bpf/local_storage.c:6: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:212:1: note: expanded from here __do_insw ^ arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw' #define __do_insw(p, b, n) readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/bpf/local_storage.c:6: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:214:1: note: expanded from here __do_insl ^ arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl' #define __do_insl(p, b, n) readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/bpf/local_storage.c:6: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:216:1: note: expanded from here __do_outsb ^ arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb' #define __do_outsb(p, b, n) writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/bpf/local_storage.c:6: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:218:1: note: expanded from here __do_outsw ^ arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw' #define __do_outsw(p, b, n) writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/bpf/local_storage.c:6: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:220:1: note: expanded from here __do_outsl ^ arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl' #define __do_outsl(p, b, n) writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ >> kernel/bpf/local_storage.c:13:33: error: use of undeclared identifier 'BPF_CGROUP_STORAGE_NEST_MAX' bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); ^ 6 warnings and 1 error generated. -- ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:200:1: note: expanded from here __do_insb ^ arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb' #define __do_insb(p, b, n) readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/trace/bpf_trace.c:11: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:202:1: note: expanded from here __do_insw ^ arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw' #define __do_insw(p, b, n) readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/trace/bpf_trace.c:11: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:204:1: note: expanded from here __do_insl ^ arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl' #define __do_insl(p, b, n) readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/trace/bpf_trace.c:11: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:206:1: note: expanded from here __do_outsb ^ arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb' #define __do_outsb(p, b, n) writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/trace/bpf_trace.c:11: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:208:1: note: expanded from here __do_outsw ^ arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw' #define __do_outsw(p, b, n) writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ In file included from kernel/trace/bpf_trace.c:11: In file included from include/linux/filter.h:13: In file included from include/linux/skbuff.h:31: In file included from include/linux/dma-mapping.h:10: In file included from include/linux/scatterlist.h:9: In file included from arch/powerpc/include/asm/io.h:619: arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET' __do_##name al; \ ^~~~~~~~~~~~~~ <scratch space>:210:1: note: expanded from here __do_outsl ^ arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl' #define __do_outsl(p, b, n) writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n)) ~~~~~~~~~~~~~~~~~~~~~^ >> kernel/trace/bpf_trace.c:127:8: error: invalid argument type 'void' to unary expression ret = BPF_PROG_RUN_ARRAY_CHECK(call->prog_array, ctx, BPF_PROG_RUN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/bpf.h:1194:2: note: expanded from macro 'BPF_PROG_RUN_ARRAY_CHECK' __BPF_PROG_RUN_ARRAY(array, ctx, func, true, false) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/bpf.h:1141:9: note: expanded from macro '__BPF_PROG_RUN_ARRAY' if (unlikely(bpf_cgroup_storage_set(_item->cgroup_storage))) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:78:40: note: expanded from macro 'unlikely' # define unlikely(x) __builtin_expect(!!(x), 0) ^~~~ >> kernel/trace/bpf_trace.c:127:8: error: implicit declaration of function 'bpf_cgroup_storage_unset' [-Werror,-Wimplicit-function-declaration] include/linux/bpf.h:1194:2: note: expanded from macro 'BPF_PROG_RUN_ARRAY_CHECK' __BPF_PROG_RUN_ARRAY(array, ctx, func, true, false) ^ include/linux/bpf.h:1144:5: note: expanded from macro '__BPF_PROG_RUN_ARRAY' bpf_cgroup_storage_unset(); \ ^ kernel/trace/bpf_trace.c:127:8: note: did you mean 'bpf_cgroup_storage_set'? include/linux/bpf.h:1194:2: note: expanded from macro 'BPF_PROG_RUN_ARRAY_CHECK' __BPF_PROG_RUN_ARRAY(array, ctx, func, true, false) ^ include/linux/bpf.h:1144:5: note: expanded from macro '__BPF_PROG_RUN_ARRAY' bpf_cgroup_storage_unset(); \ ^ include/linux/bpf-cgroup.h:492:20: note: 'bpf_cgroup_storage_set' declared here static inline void bpf_cgroup_storage_set( ^ 6 warnings and 2 errors generated. vim +/BPF_CGROUP_STORAGE_NEST_MAX +13 kernel/bpf/local_storage.c 11 12 DEFINE_PER_CPU(struct bpf_cgroup_storage_info, > 13 bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); 14 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 30670 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper 2021-03-19 0:18 ` [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper Yonghong Song 2021-03-19 1:37 ` kernel test robot @ 2021-03-19 2:40 ` kernel test robot 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2021-03-19 2:40 UTC (permalink / raw) To: Yonghong Song, bpf Cc: kbuild-all, Alexei Starovoitov, Daniel Borkmann, kernel-team, Jiri Olsa, Roman Gushchin [-- Attachment #1: Type: text/plain, Size: 2947 bytes --] Hi Yonghong, I love your patch! Yet something to improve: [auto build test ERROR on next-20210318] [cannot apply to bpf-next/master bpf/master v5.12-rc3 v5.12-rc2 v5.12-rc1 v5.12-rc3] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 base: ba5b053ab3ac674b91a6669086139819359a5e6e config: i386-randconfig-a005-20210318 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/38b56e03d083be940fe9dc231c5f6c8f4f282f15 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 git checkout 38b56e03d083be940fe9dc231c5f6c8f4f282f15 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/asm-generic/percpu.h:7, from arch/x86/include/asm/percpu.h:390, from arch/x86/include/asm/current.h:6, from arch/x86/include/asm/processor.h:17, from arch/x86/include/asm/timex.h:5, from include/linux/timex.h:65, from include/linux/time32.h:13, from include/linux/time.h:60, from include/linux/ktime.h:24, from include/linux/timer.h:6, from include/linux/workqueue.h:9, from include/linux/bpf.h:9, from include/linux/bpf-cgroup.h:5, from kernel/bpf/local_storage.c:2: >> kernel/bpf/local_storage.c:13:33: error: 'BPF_CGROUP_STORAGE_NEST_MAX' undeclared here (not in a function); did you mean 'BPF_CGROUP_STORAGE_PERCPU'? 13 | bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:104:37: note: in definition of macro 'DEFINE_PER_CPU_SECTION' 104 | __PCPU_ATTRS(sec) __typeof__(type) name | ^~~~ kernel/bpf/local_storage.c:12:1: note: in expansion of macro 'DEFINE_PER_CPU' 12 | DEFINE_PER_CPU(struct bpf_cgroup_storage_info, | ^~~~~~~~~~~~~~ vim +13 kernel/bpf/local_storage.c 11 12 DEFINE_PER_CPU(struct bpf_cgroup_storage_info, > 13 bpf_cgroup_storage_info[BPF_CGROUP_STORAGE_NEST_MAX]); 14 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 38619 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run 2021-03-19 0:17 [PATCH bpf-next 0/2] bpf: fix NULL pointer dereference in Yonghong Song 2021-03-19 0:18 ` [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper Yonghong Song @ 2021-03-19 0:18 ` Yonghong Song 2021-03-19 2:11 ` kernel test robot 1 sibling, 1 reply; 6+ messages in thread From: Yonghong Song @ 2021-03-19 0:18 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Jiri Olsa, Roman Gushchin In bpf_test_run(), check the return value of bpf_cgroup_storage_set() and do bpf_cgroup_storate_unset() properly. Cc: Jiri Olsa <jolsa@kernel.org> Cc: Roman Gushchin <guro@fb.com> Signed-off-by: Yonghong Song <yhs@fb.com> --- net/bpf/test_run.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 0abdd67f44b1..4aabf71cd95d 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -106,12 +106,16 @@ static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, bpf_test_timer_enter(&t); do { - bpf_cgroup_storage_set(storage); + ret = bpf_cgroup_storage_set(storage); + if (ret) + break; if (xdp) *retval = bpf_prog_run_xdp(prog, ctx); else *retval = BPF_PROG_RUN(prog, ctx); + + bpf_cgroup_storage_unset(); } while (bpf_test_timer_continue(&t, repeat, &ret, time)); bpf_test_timer_leave(&t); -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run 2021-03-19 0:18 ` [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run Yonghong Song @ 2021-03-19 2:11 ` kernel test robot 0 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2021-03-19 2:11 UTC (permalink / raw) To: Yonghong Song, bpf Cc: kbuild-all, clang-built-linux, Alexei Starovoitov, Daniel Borkmann, kernel-team, Jiri Olsa, Roman Gushchin [-- Attachment #1: Type: text/plain, Size: 7295 bytes --] Hi Yonghong, I love your patch! Yet something to improve: [auto build test ERROR on next-20210318] [cannot apply to bpf-next/master bpf/master v5.12-rc3 v5.12-rc2 v5.12-rc1 v5.12-rc3] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 base: ba5b053ab3ac674b91a6669086139819359a5e6e config: arm-randconfig-r014-20210318 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/0day-ci/linux/commit/4d3510a809c631ed53b78f7baa50fd4cc2730587 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Yonghong-Song/bpf-fix-NULL-pointer-dereference-in/20210319-082140 git checkout 4d3510a809c631ed53b78f7baa50fd4cc2730587 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> net/bpf/test_run.c:109:7: error: assigning to 'int' from incompatible type 'void' ret = bpf_cgroup_storage_set(storage); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> net/bpf/test_run.c:118:3: error: implicit declaration of function 'bpf_cgroup_storage_unset' [-Werror,-Wimplicit-function-declaration] bpf_cgroup_storage_unset(); ^ net/bpf/test_run.c:118:3: note: did you mean 'bpf_cgroup_storage_set'? include/linux/bpf-cgroup.h:492:20: note: 'bpf_cgroup_storage_set' declared here static inline void bpf_cgroup_storage_set( ^ net/bpf/test_run.c:167:14: warning: no previous prototype for function 'bpf_fentry_test1' [-Wmissing-prototypes] int noinline bpf_fentry_test1(int a) ^ net/bpf/test_run.c:167:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test1(int a) ^ static net/bpf/test_run.c:172:14: warning: no previous prototype for function 'bpf_fentry_test2' [-Wmissing-prototypes] int noinline bpf_fentry_test2(int a, u64 b) ^ net/bpf/test_run.c:172:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test2(int a, u64 b) ^ static net/bpf/test_run.c:177:14: warning: no previous prototype for function 'bpf_fentry_test3' [-Wmissing-prototypes] int noinline bpf_fentry_test3(char a, int b, u64 c) ^ net/bpf/test_run.c:177:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test3(char a, int b, u64 c) ^ static net/bpf/test_run.c:182:14: warning: no previous prototype for function 'bpf_fentry_test4' [-Wmissing-prototypes] int noinline bpf_fentry_test4(void *a, char b, int c, u64 d) ^ net/bpf/test_run.c:182:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test4(void *a, char b, int c, u64 d) ^ static net/bpf/test_run.c:187:14: warning: no previous prototype for function 'bpf_fentry_test5' [-Wmissing-prototypes] int noinline bpf_fentry_test5(u64 a, void *b, short c, int d, u64 e) ^ net/bpf/test_run.c:187:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test5(u64 a, void *b, short c, int d, u64 e) ^ static net/bpf/test_run.c:192:14: warning: no previous prototype for function 'bpf_fentry_test6' [-Wmissing-prototypes] int noinline bpf_fentry_test6(u64 a, void *b, short c, int d, void *e, u64 f) ^ net/bpf/test_run.c:192:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test6(u64 a, void *b, short c, int d, void *e, u64 f) ^ static net/bpf/test_run.c:201:14: warning: no previous prototype for function 'bpf_fentry_test7' [-Wmissing-prototypes] int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) ^ net/bpf/test_run.c:201:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) ^ static net/bpf/test_run.c:206:14: warning: no previous prototype for function 'bpf_fentry_test8' [-Wmissing-prototypes] int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg) ^ net/bpf/test_run.c:206:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg) ^ static net/bpf/test_run.c:211:14: warning: no previous prototype for function 'bpf_modify_return_test' [-Wmissing-prototypes] int noinline bpf_modify_return_test(int a, int *b) ^ net/bpf/test_run.c:211:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int noinline bpf_modify_return_test(int a, int *b) ^ static 9 warnings and 2 errors generated. vim +109 net/bpf/test_run.c 85 86 static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, 87 u32 *retval, u32 *time, bool xdp) 88 { 89 struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL }; 90 struct bpf_test_timer t = { NO_MIGRATE }; 91 enum bpf_cgroup_storage_type stype; 92 int ret; 93 94 for_each_cgroup_storage_type(stype) { 95 storage[stype] = bpf_cgroup_storage_alloc(prog, stype); 96 if (IS_ERR(storage[stype])) { 97 storage[stype] = NULL; 98 for_each_cgroup_storage_type(stype) 99 bpf_cgroup_storage_free(storage[stype]); 100 return -ENOMEM; 101 } 102 } 103 104 if (!repeat) 105 repeat = 1; 106 107 bpf_test_timer_enter(&t); 108 do { > 109 ret = bpf_cgroup_storage_set(storage); 110 if (ret) 111 break; 112 113 if (xdp) 114 *retval = bpf_prog_run_xdp(prog, ctx); 115 else 116 *retval = BPF_PROG_RUN(prog, ctx); 117 > 118 bpf_cgroup_storage_unset(); 119 } while (bpf_test_timer_continue(&t, repeat, &ret, time)); 120 bpf_test_timer_leave(&t); 121 122 for_each_cgroup_storage_type(stype) 123 bpf_cgroup_storage_free(storage[stype]); 124 125 return ret; 126 } 127 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 29781 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-19 2:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-19 0:17 [PATCH bpf-next 0/2] bpf: fix NULL pointer dereference in Yonghong Song 2021-03-19 0:18 ` [PATCH bpf-next 1/2] bpf: fix NULL pointer dereference in bpf_get_local_storage() helper Yonghong Song 2021-03-19 1:37 ` kernel test robot 2021-03-19 2:40 ` kernel test robot 2021-03-19 0:18 ` [PATCH bpf-next 2/2] bpf: fix bpf_cgroup_storage_set() usage in test_run Yonghong Song 2021-03-19 2:11 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox