From: Andrii Nakryiko <andrii@kernel.org>
To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
martin.lau@kernel.org
Cc: andrii@kernel.org, kernel-team@meta.com
Subject: [PATCH v3 bpf 5/5] libbpf: warn on unexpected __arg_ctx type when rewriting BTF
Date: Wed, 17 Jan 2024 19:31:43 -0800 [thread overview]
Message-ID: <20240118033143.3384355-6-andrii@kernel.org> (raw)
In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org>
On kernel that don't support arg:ctx tag, before adjusting global
subprog BTF information to match kernel's expected canonical type names,
make sure that types used by user are meaningful, and if not, warn and
don't do BTF adjustments.
This is similar to checks that kernel performs, but narrower in scope,
as only a small subset of BPF program types can be accommodated by
libbpf using canonical type names.
Libbpf unconditionally allows `struct pt_regs *` for perf_event program
types, unlike kernel, which supports that conditionally on architecture.
This is done to keep things simple and not cause unnecessary false
positives. This seems like a minor and harmless deviation, which in
real-world programs will be caught by kernels with arg:ctx tag support
anyways. So KISS principle.
This logic is hard to test (especially on latest kernels), so manual
testing was performed instead. Libbpf emitted the following warning for
perf_event program with wrong context argument type:
libbpf: prog 'arg_tag_ctx_perf': subprog 'subprog_ctx_tag' arg#0 is expected to be of `struct bpf_perf_event_data *` type
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 75 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 66 insertions(+), 9 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 61db92189517..afd09571c482 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6695,6 +6695,67 @@ static struct {
/* all other program types don't have "named" context structs */
};
+static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_program *prog,
+ const char *subprog_name, int arg_idx,
+ int arg_type_id, const char *ctx_name)
+{
+ const struct btf_type *t;
+ const char *tname;
+
+ /* check if existing parameter already matches verifier expectations */
+ t = skip_mods_and_typedefs(btf, arg_type_id, NULL);
+ if (!btf_is_ptr(t))
+ goto out_warn;
+
+ /* typedef bpf_user_pt_regs_t is a special PITA case, valid for kprobe
+ * and perf_event programs, so check this case early on and forget
+ * about it for subsequent checks
+ */
+ while (btf_is_mod(t))
+ t = btf__type_by_id(btf, t->type);
+ if (btf_is_typedef(t) &&
+ (prog->type == BPF_PROG_TYPE_KPROBE || prog->type == BPF_PROG_TYPE_PERF_EVENT)) {
+ tname = btf__str_by_offset(btf, t->name_off) ?: "<anon>";
+ if (strcmp(tname, "bpf_user_pt_regs_t") == 0)
+ return false; /* canonical type for kprobe/perf_event */
+ }
+
+ /* now we can ignore typedefs moving forward */
+ t = skip_mods_and_typedefs(btf, t->type, NULL);
+
+ /* if it's `void *`, definitely fix up BTF info */
+ if (btf_is_void(t))
+ return true;
+
+ /* if it's already proper canonical type, no need to fix up */
+ tname = btf__str_by_offset(btf, t->name_off) ?: "<anon>";
+ if (btf_is_struct(t) && strcmp(tname, ctx_name) == 0)
+ return false;
+
+ /* special cases */
+ switch (prog->type) {
+ case BPF_PROG_TYPE_KPROBE:
+ case BPF_PROG_TYPE_PERF_EVENT:
+ /* `struct pt_regs *` is expected, but we need to fix up */
+ if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
+ return true;
+ break;
+ case BPF_PROG_TYPE_RAW_TRACEPOINT:
+ case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
+ /* allow u64* as ctx */
+ if (btf_is_int(t) && t->size == 8)
+ return true;
+ break;
+ default:
+ break;
+ }
+
+out_warn:
+ pr_warn("prog '%s': subprog '%s' arg#%d is expected to be of `struct %s *` type\n",
+ prog->name, subprog_name, arg_idx, ctx_name);
+ return false;
+}
+
static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
{
int fn_id, fn_proto_id, ret_type_id, orig_proto_id;
@@ -6829,7 +6890,7 @@ static int probe_kern_arg_ctx_tag(void)
*/
static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
{
- const char *ctx_name = NULL, *ctx_tag = "arg:ctx";
+ const char *ctx_name = NULL, *ctx_tag = "arg:ctx", *fn_name;
struct bpf_func_info_min *func_rec;
struct btf_type *fn_t, *fn_proto_t;
struct btf *btf = obj->btf;
@@ -6909,15 +6970,11 @@ static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_progra
if (arg_idx < 0 || arg_idx >= arg_cnt)
continue;
- /* check if existing parameter already matches verifier expectations */
+ /* check if we should fix up argument type */
p = &btf_params(fn_proto_t)[arg_idx];
- t = skip_mods_and_typedefs(btf, p->type, NULL);
- if (btf_is_ptr(t) &&
- (t = skip_mods_and_typedefs(btf, t->type, NULL)) &&
- btf_is_struct(t) &&
- strcmp(btf__str_by_offset(btf, t->name_off), ctx_name) == 0) {
- continue; /* no need for fix up */
- }
+ fn_name = btf__str_by_offset(btf, fn_t->name_off) ?: "<anon>";
+ if (!need_func_arg_type_fixup(btf, prog, fn_name, arg_idx, p->type, ctx_name))
+ continue;
/* clone fn/fn_proto, unless we already did it for another arg */
if (func_rec->type_id == orig_fn_id) {
--
2.34.1
next prev parent reply other threads:[~2024-01-18 3:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-18 3:31 [PATCH v3 bpf 0/5] Tighten up arg:ctx type enforcement Andrii Nakryiko
2024-01-18 3:31 ` [PATCH v3 bpf 1/5] libbpf: feature-detect arg:ctx tag support in kernel Andrii Nakryiko
2024-01-18 3:31 ` [PATCH v3 bpf 2/5] bpf: extract bpf_ctx_convert_map logic and make it more reusable Andrii Nakryiko
2024-01-18 3:31 ` [PATCH v3 bpf 3/5] bpf: enforce types for __arg_ctx-tagged arguments in global subprogs Andrii Nakryiko
2024-01-18 3:31 ` [PATCH v3 bpf 4/5] selftests/bpf: add tests confirming type logic in kernel for __arg_ctx Andrii Nakryiko
2024-01-18 3:31 ` Andrii Nakryiko [this message]
2024-01-18 4:30 ` [PATCH v3 bpf 0/5] Tighten up arg:ctx type enforcement patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240118033143.3384355-6-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@meta.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox