| From ee9a803a5a3f4af7baaac8ffb5afb96abdb53d71 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 7 Jul 2023 16:11:56 -0700 |
| Subject: libbpf: only reset sec_def handler when necessary |
| |
| From: Andrii Nakryiko <andrii@kernel.org> |
| |
| [ Upstream commit c628747cc8800cf6d33d09f7f42c8b6f91e64dc7 ] |
| |
| Don't reset recorded sec_def handler unconditionally on |
| bpf_program__set_type(). There are two situations where this is wrong. |
| |
| First, if the program type didn't actually change. In that case original |
| SEC handler should work just fine. |
| |
| Second, catch-all custom SEC handler is supposed to work with any BPF |
| program type and SEC() annotation, so it also doesn't make sense to |
| reset that. |
| |
| This patch fixes both issues. This was reported recently in the context |
| of breaking perf tool, which uses custom catch-all handler for fancy BPF |
| prologue generation logic. This patch should fix the issue. |
| |
| [0] https://lore.kernel.org/linux-perf-users/ab865e6d-06c5-078e-e404-7f90686db50d@amd.com/ |
| |
| Fixes: d6e6286a12e7 ("libbpf: disassociate section handler on explicit bpf_program__set_type() call") |
| Reported-by: Ravi Bangoria <ravi.bangoria@amd.com> |
| Signed-off-by: Andrii Nakryiko <andrii@kernel.org> |
| Acked-by: Stanislav Fomichev <sdf@google.com> |
| Link: https://lore.kernel.org/r/20230707231156.1711948-1-andrii@kernel.org |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| tools/lib/bpf/libbpf.c | 27 +++++++++++++++++++-------- |
| 1 file changed, 19 insertions(+), 8 deletions(-) |
| |
| diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c |
| index a27f6e9ccce75..57c040a9c3705 100644 |
| --- a/tools/lib/bpf/libbpf.c |
| +++ b/tools/lib/bpf/libbpf.c |
| @@ -8534,13 +8534,31 @@ enum bpf_prog_type bpf_program__type(const struct bpf_program *prog) |
| return prog->type; |
| } |
| |
| +static size_t custom_sec_def_cnt; |
| +static struct bpf_sec_def *custom_sec_defs; |
| +static struct bpf_sec_def custom_fallback_def; |
| +static bool has_custom_fallback_def; |
| +static int last_custom_sec_def_handler_id; |
| + |
| int bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type) |
| { |
| if (prog->obj->loaded) |
| return libbpf_err(-EBUSY); |
| |
| + /* if type is not changed, do nothing */ |
| + if (prog->type == type) |
| + return 0; |
| + |
| prog->type = type; |
| - prog->sec_def = NULL; |
| + |
| + /* If a program type was changed, we need to reset associated SEC() |
| + * handler, as it will be invalid now. The only exception is a generic |
| + * fallback handler, which by definition is program type-agnostic and |
| + * is a catch-all custom handler, optionally set by the application, |
| + * so should be able to handle any type of BPF program. |
| + */ |
| + if (prog->sec_def != &custom_fallback_def) |
| + prog->sec_def = NULL; |
| return 0; |
| } |
| |
| @@ -8716,13 +8734,6 @@ static const struct bpf_sec_def section_defs[] = { |
| SEC_DEF("netfilter", NETFILTER, BPF_NETFILTER, SEC_NONE), |
| }; |
| |
| -static size_t custom_sec_def_cnt; |
| -static struct bpf_sec_def *custom_sec_defs; |
| -static struct bpf_sec_def custom_fallback_def; |
| -static bool has_custom_fallback_def; |
| - |
| -static int last_custom_sec_def_handler_id; |
| - |
| int libbpf_register_prog_handler(const char *sec, |
| enum bpf_prog_type prog_type, |
| enum bpf_attach_type exp_attach_type, |
| -- |
| 2.40.1 |
| |