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 v2 bpf-next 3/4] bpf: don't infer PTR_TO_CTX for programs with unnamed context type
Date: Mon, 12 Feb 2024 15:32:20 -0800 [thread overview]
Message-ID: <20240212233221.2575350-4-andrii@kernel.org> (raw)
In-Reply-To: <20240212233221.2575350-1-andrii@kernel.org>
For program types that don't have named context type name (e.g., BPF
iterator programs or tracepoint programs), ctx_tname will be a non-NULL
empty string. For such programs it shouldn't be possible to have
PTR_TO_CTX argument for global subprogs based on type name alone.
arg:ctx tag is the only way to have PTR_TO_CTX passed into global
subprog for such program types.
Fix this loophole, which currently would assume PTR_TO_CTX whenever
user uses a pointer to anonymous struct as an argument to their global
subprogs. This happens in practice with the following (quite common, in
practice) approach:
typedef struct { /* anonymous */
int x;
} my_type_t;
int my_subprog(my_type_t *arg) { ... }
User's intent is to have PTR_TO_MEM argument for `arg`, but verifier
will complain about expecting PTR_TO_CTX.
This fix also closes unintended s390x-specific KPROBE handling of
PTR_TO_CTX case. Selftest change is necessary to accommodate this.
Fixes: 91cc1a99740e ("bpf: Annotate context types")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/btf.c | 3 +++
.../bpf/progs/test_global_func_ctx_args.c | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index da958092c969..13bd93efeed0 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5739,6 +5739,9 @@ bool btf_is_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
bpf_log(log, "Please fix kernel include/linux/bpf_types.h\n");
return false;
}
+ /* program types without named context types work only with arg:ctx tag */
+ if (ctx_tname[0] == '\0')
+ return false;
/* only compare that prog's ctx type name is the same as
* kernel expects. No need to compare field by field.
* It's ok for bpf prog to do:
diff --git a/tools/testing/selftests/bpf/progs/test_global_func_ctx_args.c b/tools/testing/selftests/bpf/progs/test_global_func_ctx_args.c
index 9a06e5eb1fbe..143c8a4852bf 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func_ctx_args.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func_ctx_args.c
@@ -26,6 +26,23 @@ int kprobe_typedef_ctx(void *ctx)
return kprobe_typedef_ctx_subprog(ctx);
}
+/* s390x defines:
+ *
+ * typedef user_pt_regs bpf_user_pt_regs_t;
+ * typedef struct { ... } user_pt_regs;
+ *
+ * And so "canonical" underlying struct type is anonymous.
+ * So on s390x only valid ways to have PTR_TO_CTX argument in global subprogs
+ * are:
+ * - bpf_user_pt_regs_t *ctx (typedef);
+ * - struct bpf_user_pt_regs_t *ctx (backwards compatible struct hack);
+ * - void *ctx __arg_ctx (arg:ctx tag)
+ *
+ * Other architectures also allow using underlying struct types (e.g.,
+ * `struct pt_regs *ctx` for x86-64)
+ */
+#ifndef bpf_target_s390
+
#define pt_regs_struct_t typeof(*(__PT_REGS_CAST((struct pt_regs *)NULL)))
__weak int kprobe_struct_ctx_subprog(pt_regs_struct_t *ctx)
@@ -40,6 +57,8 @@ int kprobe_resolved_ctx(void *ctx)
return kprobe_struct_ctx_subprog(ctx);
}
+#endif
+
/* this is current hack to make this work on old kernels */
struct bpf_user_pt_regs_t {};
--
2.39.3
next prev parent reply other threads:[~2024-02-12 23:32 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-12 23:32 [PATCH v2 bpf-next 0/4] Fix global subprog PTR_TO_CTX arg handling Andrii Nakryiko
2024-02-12 23:32 ` [PATCH v2 bpf-next 1/4] bpf: simplify btf_get_prog_ctx_type() into btf_is_prog_ctx_type() Andrii Nakryiko
2024-02-12 23:32 ` [PATCH v2 bpf-next 2/4] bpf: handle bpf_user_pt_regs_t typedef explicitly for PTR_TO_CTX global arg Andrii Nakryiko
2024-02-13 16:40 ` Eduard Zingerman
2024-02-13 17:02 ` Andrii Nakryiko
2024-02-13 17:08 ` Eduard Zingerman
2024-02-13 18:12 ` Andrii Nakryiko
2024-02-13 18:48 ` Eduard Zingerman
2024-02-13 18:59 ` Andrii Nakryiko
2024-02-12 23:32 ` Andrii Nakryiko [this message]
2024-02-12 23:32 ` [PATCH v2 bpf-next 4/4] selftests/bpf: add anonymous user struct as global subprog arg test Andrii Nakryiko
2024-02-13 12:51 ` [PATCH v2 bpf-next 0/4] Fix global subprog PTR_TO_CTX arg handling Jiri Olsa
2024-02-13 16:39 ` Eduard Zingerman
2024-02-14 2:50 ` 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=20240212233221.2575350-4-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