[PATCH bpf-next 1/2] bpf: don't infer PTR_TO_CTX for programs with unnamed context type

[PATCH bpf-next 1/2] bpf: don't infer PTR_TO_CTX for programs with unnamed context type

[Andrii Nakryiko]

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 loop hole, 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.

Fixes: 91cc1a99740e ("bpf: Annotate context types")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/btf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8e06d29961f1..d6021290caba 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5725,6 +5725,9 @@ btf_get_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 NULL;
 	}
+	/* program types without named context types work only with arg:ctx tag */
+	if (ctx_tname[0] == '\0')
+		return NULL;
 	/* 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:
-- 
2.39.3

[PATCH bpf-next 2/2] selftests/bpf: add anonymous user struct as global subprog arg test

[Andrii Nakryiko]

Add tests validating that kernel handles pointer to anonymous struct
argument as PTR_TO_MEM case, not as PTR_TO_CTX case.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../bpf/progs/verifier_global_subprogs.c      | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
index 67dddd941891..fed847bc1911 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
@@ -115,6 +115,35 @@ int arg_tag_nullable_ptr_fail(void *ctx)
 	return subprog_nullable_ptr_bad(&x);
 }
 
+typedef struct {
+	int x;
+} user_struct_t;
+
+__noinline __weak int subprog_user_anon_mem(user_struct_t *t)
+{
+	return t ? t->x : 0;
+}
+
+SEC("?tracepoint")
+__failure __log_level(2)
+__msg("invalid bpf_context access")
+__msg("Caller passes invalid args into func#1 ('subprog_user_anon_mem')")
+int anon_user_mem_invalid(void *ctx)
+{
+	/* can't pass PTR_TO_CTX as user memory */
+	return subprog_user_anon_mem(ctx) & 1;
+}
+
+SEC("?tracepoint")
+__success __log_level(2)
+__msg("Func#1 ('subprog_user_anon_mem') is safe for any args that match its prototype")
+int anon_user_mem_valid(void *ctx)
+{
+	user_struct_t t = { .x = 42 };
+
+	return subprog_user_anon_mem(&t);
+}
+
 __noinline __weak int subprog_nonnull_ptr_good(int *p1 __arg_nonnull, int *p2 __arg_nonnull)
 {
 	return (*p1) * (*p2); /* good, no need for NULL checks */
-- 
2.39.3

Re: [PATCH bpf-next 1/2] bpf: don't infer PTR_TO_CTX for programs with unnamed context type

[Andrii Nakryiko]

On Fri, Feb 9, 2024 at 3:09 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> 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 loop hole, 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.
>
> Fixes: 91cc1a99740e ("bpf: Annotate context types")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  kernel/bpf/btf.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8e06d29961f1..d6021290caba 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -5725,6 +5725,9 @@ btf_get_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 NULL;
>         }
> +       /* program types without named context types work only with arg:ctx tag */
> +       if (ctx_tname[0] == '\0')
> +               return NULL;

this break s390 because there `bpf_user_pt_regs_t *ctx` was supported
not based on `bpf_user_pt_regs_t` name, but because bpf_user_pt_regs_t
is actually a typedef to anonymous struct... (i.e., by accident). I'll
think about how to fix s390 and will post v2 next week.

>         /* 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:
> --
> 2.39.3
>