Re: [PATCH v2 bpf-next 2/4] bpf: handle bpf_user_pt_regs_t typedef explicitly for PTR_TO_CTX global arg

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-02-12 at 15:32 -0800, Andrii Nakryiko wrote:
> Expected canonical argument type for global function arguments
> representing PTR_TO_CTX is `bpf_user_pt_regs_t *ctx`. This currently
> works on s390x by accident because kernel resolves such typedef to
> underlying struct (which is anonymous on s390x), and erroneously
> accepting it as expected context type. We are fixing this problem next,
> which would break s390x arch, so we need to handle `bpf_user_pt_regs_t`
> case explicitly for KPROBE programs.
> 
> Fixes: 91cc1a99740e ("bpf: Annotate context types")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Nit: same could be achieved w/o special casing kprobes by looking
     if typedef's type is named before skipping, e.g. as below.
     But I do not insist, probably good as it is as well.

---

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f0ce384aa73e..830635b37fa1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -907,11 +907,9 @@ bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
 }
 
 /* Similar to btf_type_skip_modifiers() but does not skip typedefs. */
-static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
-						       u32 id)
+static const struct btf_type *__btf_type_skip_qualifiers(const struct btf *btf,
+							 const struct btf_type *t)
 {
-	const struct btf_type *t = btf_type_by_id(btf, id);
-
 	while (btf_type_is_modifier(t) &&
 	       BTF_INFO_KIND(t->info) != BTF_KIND_TYPEDEF) {
 		t = btf_type_by_id(btf, t->type);
@@ -920,6 +918,12 @@ static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
 	return t;
 }
 
+static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
+						       u32 id)
+{
+	return __btf_type_skip_qualifiers(btf, btf_type_by_id(btf, id));
+}
+
 #define BTF_SHOW_MAX_ITER	10
 
 #define BTF_KIND_BIT(kind)	(1ULL << kind)
@@ -5695,9 +5699,25 @@ bool btf_is_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
 	const char *tname, *ctx_tname;
 
 	t = btf_type_by_id(btf, t->type);
-	while (btf_type_is_modifier(t))
-		t = btf_type_by_id(btf, t->type);
-	if (!btf_type_is_struct(t)) {
+
+	/* Skip modifiers, but stop if skipping of typedef would
+	 * lead an anonymous type, e.g. like for s390:
+	 *
+	 *   typedef struct { ... } user_pt_regs;
+	 *   typedef user_pt_regs bpf_user_pt_regs_t;
+	 */
+	t = __btf_type_skip_qualifiers(btf, t);
+	while (btf_type_is_typedef(t)) {
+		const struct btf_type *t1;
+
+		t1 = btf_type_by_id(btf, t->type);
+		t1 = __btf_type_skip_qualifiers(btf, t1);
+		tname = btf_name_by_offset(btf, t1->name_off);
+		if (!tname || tname[0] == '\0')
+			break;
+		t = t1;
+	}
+	if (!btf_type_is_struct(t) && !btf_type_is_typedef(t)) {
 		/* Only pointer to struct is supported for now.
 		 * That means that BPF_PROG_TYPE_TRACEPOINT with BTF
 		 * is not supported yet.