Re: [PATCH bpf-next v2 03/18] bpf: Split kfunc map argument into __const_map and __map

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-07-24 at 12:07 -0700, Amery Hung wrote:
> Kfuncs used a single '__map' suffix (KF_ARG_PTR_TO_MAP) for two different
> things: a verifier-known map matched by map_uid against a bound
> timer/wq/task_work object (bpf_wq_init, bpf_task_work_schedule*), and an
> opaque 'struct bpf_map *' used only at runtime (bpf_arena_*), which may be a
> map fd or a PTR_TO_BTF_ID struct bpf_map (e.g. a bpf_map iterator's ctx->map).
> 
> That combined path only accepted the btf map form due to type confusion. The
> 'if (!reg->map_ptr)' check reads reg->map_ptr, which aliases reg->btf in the
> bpf_reg_state union. A PTR_TO_BTF_ID register always has a non-NULL reg->btf,
> so the guard silently passed and validation fell through to
> process_kf_arg_ptr_to_btf_id(). It also recorded PTR_TO_BTF_ID info in
> meta->map, which would be meaningless.

What a bug, I wonder if we have more places like that.
In LLVM code base they use the following pattern:

  class foo {
    int tag;
    union {
      struct bar_info { ... } bar;
      struct buz_info { ... } buz;
    };
  };

  struct bar_info *get_bar(class foo *f) {
    if (f->tag != BAR)
      fatal_error(); // the error is compiled away in release builds
    return &f->bar;
  }

And similar pattern with checks in getter for just mutually exclusive
fields. I wonder if we should adopt the same approach.

> Split the annotation to avoid such type confusion and to align with
> helper:
> 
> - '__const_map' -> KF_ARG_CONST_MAP_PTR: verifier-known map, handled by
>   process_map_ptr_arg() like helper ARG_CONST_MAP_PTR.
> 
> - '__map' -> KF_ARG_PTR_TO_BTF_ID: opaque struct bpf_map, validated by
>   process_kf_arg_ptr_to_btf_id(). A map fd still matches via
>   reg2btf_ids[CONST_PTR_TO_MAP], so bpf_arena_alloc_pages(&map) keeps
>   working.
> 
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>