On Tue, Mar 09, 2021 at 10:50:28AM -0800, Yonghong Song wrote:
> 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@xxxxxxxxxxxxxx/T
>
> Cc: Roman Gushchin <guro@xxxxxx>
> Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
Acked-by: Roman Gushchin <guro@xxxxxx>
Thanks, Yonghong!
