Re: [PATCH v3 bpf-next 7/9] libbpf: implement __arg_ctx fallback logic

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Wed, Jan 3, 2024 at 5:39 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
>
> This limitation was the reason to add btf_decl_tag("arg:ctx"), making
> the actual argument type not important, so that user can just define
> "generic" signature:
>
>   __noinline int global_subprog(void *ctx __arg_ctx) { ... }

I still think that this __arg_ctx only makes sense with 'void *'.
Blind rewrite of ctx is a foot gun.

I've tried the following:

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..0e5f5205d4a8 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
@@ -106,9 +106,9 @@ int perf_event_ctx(void *ctx)
 /* this global subprog can be now called from many types of entry progs, each
  * with different context type
  */
-__weak int subprog_ctx_tag(void *ctx __arg_ctx)
+__weak int subprog_ctx_tag(long ctx __arg_ctx)
 {
-       return bpf_get_stack(ctx, stack, sizeof(stack), 0);
+       return bpf_get_stack((void *)ctx, stack, sizeof(stack), 0);
 }

 struct my_struct { int x; };
@@ -131,7 +131,7 @@ int arg_tag_ctx_raw_tp(void *ctx)
 {
        struct my_struct x = { .x = 123 };

-       return subprog_ctx_tag(ctx) + subprog_multi_ctx_tags(ctx, &x, ctx);
+       return subprog_ctx_tag((long)ctx) +
subprog_multi_ctx_tags(ctx, &x, ctx);
 }

and it "works".

Both kernel and libbpf should really limit it to 'void *'.

In the other email I suggested to allow types that match expected
based on prog type, but even that is probably a danger zone as well.
The correct type would already be detected by the verifier,
so extra __arg_ctx is pointless.
It makes sense only for such polymorphic functions and those
better use 'void *' and don't dereference it.

I think this can be a follow up.