* [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT @ 2022-03-30 1:14 Martin KaFai Lau 2022-03-30 1:15 ` [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program Martin KaFai Lau ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Martin KaFai Lau @ 2022-03-30 1:14 UTC (permalink / raw) To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team The commit 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications") fixes the verifier checking for BPF_PROG_TYPE_EXT (extension) prog such that the verifier looks for things based on the target prog type that it is extending instead of the BPF_PROG_TYPE_EXT itself. The current resolve_prog_type() returns the target prog type. It checks for nullness on prog->aux->dst_prog. However, when loading a BPF_PROG_TYPE_TRACING prog and it is tracing another bpf prog instead of a kernel function, prog->aux->dst_prog is not NULL also. In this case, the verifier should still verify as the BPF_PROG_TYPE_TRACING type instead of the traced prog type in prog->aux->dst_prog->type. An oops has been reported when tracing a struct_ops prog. A NULL dereference happened in check_return_code() when accessing the prog->aux->attach_func_proto->type and prog->aux->attach_func_proto is NULL here because the traced struct_ops prog has the "unreliable" set. This patch is to change the resolve_prog_type() to only return the target prog type if the prog being verified is BPF_PROG_TYPE_EXT. Fixes: 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications") Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- include/linux/bpf_verifier.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index c1fc4af47f69..3a9d2d7cc6b7 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -570,9 +570,11 @@ static inline u32 type_flag(u32 type) return type & ~BPF_BASE_TYPE_MASK; } +/* only use after check_attach_btf_id() */ static inline enum bpf_prog_type resolve_prog_type(struct bpf_prog *prog) { - return prog->aux->dst_prog ? prog->aux->dst_prog->type : prog->type; + return prog->type == BPF_PROG_TYPE_EXT ? + prog->aux->dst_prog->type : prog->type; } #endif /* _LINUX_BPF_VERIFIER_H */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program 2022-03-30 1:14 [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Martin KaFai Lau @ 2022-03-30 1:15 ` Martin KaFai Lau 2022-03-30 18:25 ` Yonghong Song 2022-03-30 5:25 ` [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Yonghong Song 2022-03-31 2:40 ` patchwork-bot+netdevbpf 2 siblings, 1 reply; 5+ messages in thread From: Martin KaFai Lau @ 2022-03-30 1:15 UTC (permalink / raw) To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team This patch tests attaching an fentry prog to a struct_ops prog. Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- .../selftests/bpf/prog_tests/dummy_st_ops.c | 23 +++++++++++++++++++ .../selftests/bpf/progs/trace_dummy_st_ops.c | 21 +++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/trace_dummy_st_ops.c diff --git a/tools/testing/selftests/bpf/prog_tests/dummy_st_ops.c b/tools/testing/selftests/bpf/prog_tests/dummy_st_ops.c index 5aa52cc31dc2..c11832657d2b 100644 --- a/tools/testing/selftests/bpf/prog_tests/dummy_st_ops.c +++ b/tools/testing/selftests/bpf/prog_tests/dummy_st_ops.c @@ -2,6 +2,7 @@ /* Copyright (C) 2021. Huawei Technologies Co., Ltd */ #include <test_progs.h> #include "dummy_st_ops.skel.h" +#include "trace_dummy_st_ops.skel.h" /* Need to keep consistent with definition in include/linux/bpf.h */ struct bpf_dummy_ops_state { @@ -56,6 +57,7 @@ static void test_dummy_init_ptr_arg(void) .ctx_in = args, .ctx_size_in = sizeof(args), ); + struct trace_dummy_st_ops *trace_skel; struct dummy_st_ops *skel; int fd, err; @@ -64,12 +66,33 @@ static void test_dummy_init_ptr_arg(void) return; fd = bpf_program__fd(skel->progs.test_1); + + trace_skel = trace_dummy_st_ops__open(); + if (!ASSERT_OK_PTR(trace_skel, "trace_dummy_st_ops__open")) + goto done; + + err = bpf_program__set_attach_target(trace_skel->progs.fentry_test_1, + fd, "test_1"); + if (!ASSERT_OK(err, "set_attach_target(fentry_test_1)")) + goto done; + + err = trace_dummy_st_ops__load(trace_skel); + if (!ASSERT_OK(err, "load(trace_skel)")) + goto done; + + err = trace_dummy_st_ops__attach(trace_skel); + if (!ASSERT_OK(err, "attach(trace_skel)")) + goto done; + err = bpf_prog_test_run_opts(fd, &attr); ASSERT_OK(err, "test_run"); ASSERT_EQ(in_state.val, 0x5a, "test_ptr_ret"); ASSERT_EQ(attr.retval, exp_retval, "test_ret"); + ASSERT_EQ(trace_skel->bss->val, exp_retval, "fentry_val"); +done: dummy_st_ops__destroy(skel); + trace_dummy_st_ops__destroy(trace_skel); } static void test_dummy_multiple_args(void) diff --git a/tools/testing/selftests/bpf/progs/trace_dummy_st_ops.c b/tools/testing/selftests/bpf/progs/trace_dummy_st_ops.c new file mode 100644 index 000000000000..00a4be9d3074 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/trace_dummy_st_ops.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +int val = 0; + +SEC("fentry/test_1") +int BPF_PROG(fentry_test_1, __u64 *st_ops_ctx) +{ + __u64 state; + + /* Read the traced st_ops arg1 which is a pointer */ + bpf_probe_read_kernel(&state, sizeof(__u64), (void *)st_ops_ctx); + /* Read state->val */ + bpf_probe_read_kernel(&val, sizeof(__u32), (void *)state); + + return 0; +} + +char _license[] SEC("license") = "GPL"; -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program 2022-03-30 1:15 ` [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program Martin KaFai Lau @ 2022-03-30 18:25 ` Yonghong Song 0 siblings, 0 replies; 5+ messages in thread From: Yonghong Song @ 2022-03-30 18:25 UTC (permalink / raw) To: Martin KaFai Lau, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team On 3/29/22 6:15 PM, Martin KaFai Lau wrote: > This patch tests attaching an fentry prog to a struct_ops prog. > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> Acked-by: Yonghong Song <yhs@fb.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT 2022-03-30 1:14 [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Martin KaFai Lau 2022-03-30 1:15 ` [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program Martin KaFai Lau @ 2022-03-30 5:25 ` Yonghong Song 2022-03-31 2:40 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 5+ messages in thread From: Yonghong Song @ 2022-03-30 5:25 UTC (permalink / raw) To: Martin KaFai Lau, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team On 3/29/22 6:14 PM, Martin KaFai Lau wrote: > The commit 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications") > fixes the verifier checking for BPF_PROG_TYPE_EXT (extension) > prog such that the verifier looks for things based > on the target prog type that it is extending instead of > the BPF_PROG_TYPE_EXT itself. > > The current resolve_prog_type() returns the target prog type. > It checks for nullness on prog->aux->dst_prog. However, > when loading a BPF_PROG_TYPE_TRACING prog and it is tracing another > bpf prog instead of a kernel function, prog->aux->dst_prog is not > NULL also. In this case, the verifier should still verify as the > BPF_PROG_TYPE_TRACING type instead of the traced prog type in > prog->aux->dst_prog->type. > > An oops has been reported when tracing a struct_ops prog. A NULL > dereference happened in check_return_code() when accessing the > prog->aux->attach_func_proto->type and prog->aux->attach_func_proto > is NULL here because the traced struct_ops prog has the "unreliable" set. > > This patch is to change the resolve_prog_type() to only > return the target prog type if the prog being verified is > BPF_PROG_TYPE_EXT. > > Fixes: 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications") > Signed-off-by: Martin KaFai Lau <kafai@fb.com> Acked-by: Yonghong Song <yhs@fb.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT 2022-03-30 1:14 [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Martin KaFai Lau 2022-03-30 1:15 ` [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program Martin KaFai Lau 2022-03-30 5:25 ` [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Yonghong Song @ 2022-03-31 2:40 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 5+ messages in thread From: patchwork-bot+netdevbpf @ 2022-03-31 2:40 UTC (permalink / raw) To: Martin KaFai Lau; +Cc: bpf, ast, andrii, daniel, kernel-team Hello: This series was applied to bpf/bpf.git (master) by Alexei Starovoitov <ast@kernel.org>: On Tue, 29 Mar 2022 18:14:56 -0700 you wrote: > The commit 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications") > fixes the verifier checking for BPF_PROG_TYPE_EXT (extension) > prog such that the verifier looks for things based > on the target prog type that it is extending instead of > the BPF_PROG_TYPE_EXT itself. > > The current resolve_prog_type() returns the target prog type. > It checks for nullness on prog->aux->dst_prog. However, > when loading a BPF_PROG_TYPE_TRACING prog and it is tracing another > bpf prog instead of a kernel function, prog->aux->dst_prog is not > NULL also. In this case, the verifier should still verify as the > BPF_PROG_TYPE_TRACING type instead of the traced prog type in > prog->aux->dst_prog->type. > > [...] Here is the summary with links: - [bpf,1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT https://git.kernel.org/bpf/bpf/c/4a9c7bbe2ed4 - [bpf,2/2] bpf: selftests: Test fentry tracing a struct_ops program https://git.kernel.org/bpf/bpf/c/0a210af6d0a0 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-31 2:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-30 1:14 [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Martin KaFai Lau 2022-03-30 1:15 ` [PATCH bpf 2/2] bpf: selftests: Test fentry tracing a struct_ops program Martin KaFai Lau 2022-03-30 18:25 ` Yonghong Song 2022-03-30 5:25 ` [PATCH bpf 1/2] bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT Yonghong Song 2022-03-31 2:40 ` patchwork-bot+netdevbpf
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).