| From ac1f61b52a3ac5b2480dd3ca38137be966b26962 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sat, 27 Mar 2021 18:27:53 -0400 |
| Subject: bpf: Don't do bpf_cgroup_storage_set() for kuprobe/tp programs |
| |
| [ Upstream commit 05a68ce5fa51a83c360381630f823545c5757aa2 ] |
| |
| For kuprobe and tracepoint bpf programs, kernel calls |
| trace_call_bpf() which calls BPF_PROG_RUN_ARRAY_CHECK() |
| to run the program array. Currently, BPF_PROG_RUN_ARRAY_CHECK() |
| also calls bpf_cgroup_storage_set() to set percpu |
| cgroup local storage with NULL value. This is |
| due to Commit 394e40a29788 ("bpf: extend bpf_prog_array to store |
| pointers to the cgroup storage") which modified |
| __BPF_PROG_RUN_ARRAY() to call bpf_cgroup_storage_set() |
| and this macro is also used by BPF_PROG_RUN_ARRAY_CHECK(). |
| |
| kuprobe and tracepoint programs are not allowed to call |
| bpf_get_local_storage() helper hence does not |
| access percpu cgroup local storage. Let us |
| change BPF_PROG_RUN_ARRAY_CHECK() not to |
| modify percpu cgroup local storage. |
| |
| The issue is observed when I tried to debug [1] where |
| percpu data is overwritten due to |
| preempt_disable -> migration_disable |
| change. This patch does not completely fix the above issue, |
| which will be addressed separately, e.g., multiple cgroup |
| prog runs may preempt each other. But it does fix |
| any potential issue caused by tracing program |
| overwriting percpu cgroup storage: |
| - in a busy system, a tracing program is to run between |
| bpf_cgroup_storage_set() and the cgroup prog run. |
| - a kprobe program is triggered by a helper in cgroup prog |
| before bpf_get_local_storage() is called. |
| |
| [1] https://lore.kernel.org/bpf/CAKH8qBuXCfUz=w8L+Fj74OaUpbosO29niYwTki7e3Ag044_aww@mail.gmail.com/T |
| |
| Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage") |
| Signed-off-by: Yonghong Song <yhs@fb.com> |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Acked-by: Roman Gushchin <guro@fb.com> |
| Link: https://lore.kernel.org/bpf/20210309185028.3763817-1-yhs@fb.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| include/linux/bpf.h | 9 +++++---- |
| 1 file changed, 5 insertions(+), 4 deletions(-) |
| |
| diff --git a/include/linux/bpf.h b/include/linux/bpf.h |
| index 007147f64390..66590ae89c97 100644 |
| --- a/include/linux/bpf.h |
| +++ b/include/linux/bpf.h |
| @@ -535,7 +535,7 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array, |
| struct bpf_prog *include_prog, |
| struct bpf_prog_array **new_array); |
| |
| -#define __BPF_PROG_RUN_ARRAY(array, ctx, func, check_non_null) \ |
| +#define __BPF_PROG_RUN_ARRAY(array, ctx, func, check_non_null, set_cg_storage) \ |
| ({ \ |
| struct bpf_prog_array_item *_item; \ |
| struct bpf_prog *_prog; \ |
| @@ -548,7 +548,8 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array, |
| goto _out; \ |
| _item = &_array->items[0]; \ |
| while ((_prog = READ_ONCE(_item->prog))) { \ |
| - bpf_cgroup_storage_set(_item->cgroup_storage); \ |
| + if (set_cg_storage) \ |
| + bpf_cgroup_storage_set(_item->cgroup_storage); \ |
| _ret &= func(_prog, ctx); \ |
| _item++; \ |
| } \ |
| @@ -609,10 +610,10 @@ _out: \ |
| }) |
| |
| #define BPF_PROG_RUN_ARRAY(array, ctx, func) \ |
| - __BPF_PROG_RUN_ARRAY(array, ctx, func, false) |
| + __BPF_PROG_RUN_ARRAY(array, ctx, func, false, true) |
| |
| #define BPF_PROG_RUN_ARRAY_CHECK(array, ctx, func) \ |
| - __BPF_PROG_RUN_ARRAY(array, ctx, func, true) |
| + __BPF_PROG_RUN_ARRAY(array, ctx, func, true, false) |
| |
| #ifdef CONFIG_BPF_SYSCALL |
| DECLARE_PER_CPU(int, bpf_prog_active); |
| -- |
| 2.30.1 |
| |