Re: [PATCH bpf-next v3 3/3] selftests/bpf: add test cases with CONST_PTR_TO_MAP null checks

[Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>]

On Wed, Jun 4, 2025 at 3:28 PM Ihor Solodrai <isolodrai@xxxxxxxx> wrote:
>
> A test requires the following to happen:
>   * CONST_PTR_TO_MAP value is put on the stack
>   * then this value is checked for null
>   * the code in the null branch fails verification
>
> I was able to achieve this by using a stack allocated array of maps,
> populated with values from a global map. This is the first test case:
> map_ptr_is_never_null.
>
> The second test case (map_ptr_is_never_null_rb) involves an array of
> ringbufs and attempts to recreate a common coding pattern [1].
>
> [1] https://lore.kernel.org/bpf/CAEf4BzZNU0gX_sQ8k8JaLe1e+Veth3Rk=4x7MDhv=hQxvO8EDw@xxxxxxxxxxxxxx/
>
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Ihor Solodrai <isolodrai@xxxxxxxx>
> ---
>  .../selftests/bpf/progs/verifier_map_in_map.c | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>

LGTM overall

Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>

> diff --git a/tools/testing/selftests/bpf/progs/verifier_map_in_map.c b/tools/testing/selftests/bpf/progs/verifier_map_in_map.c
> index 7d088ba99ea5..1dd5c6902c53 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_map_in_map.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_map_in_map.c
> @@ -139,4 +139,81 @@ __naked void on_the_inner_map_pointer(void)
>         : __clobber_all);
>  }
>
> +SEC("socket")
> +int map_ptr_is_never_null(void *ctx)
> +{
> +       struct bpf_map *maps[2] = { 0 };
> +       struct bpf_map *map = NULL;
> +       int __attribute__((aligned(8))) key = 0;

aligned(8) makes any difference?

> +
> +       for (key = 0; key < 2; key++) {
> +               map = bpf_map_lookup_elem(&map_in_map, &key);
> +               if (map)
> +                       maps[key] = map;
> +               else
> +                       return 0;
> +       }
> +
> +       /* After the loop every element of maps is CONST_PTR_TO_MAP so
> +        * the invalid branch should not be explored by the verifier.
> +        */
> +       if (!maps[0])
> +               asm volatile ("r10 = 0;");
> +
> +       return 0;
> +}
> +
> +struct {
> +       __uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
> +       __uint(max_entries, 1);
> +       __type(key, int);
> +       __type(value, int);
> +       __array(values, struct {
> +               __uint(type, BPF_MAP_TYPE_RINGBUF);
> +               __uint(max_entries, 4096);

nit: use 64 * 1024 just in case, for arches where page size is 64KB

> +       });
> +} rb_in_map SEC(".maps");
> +
> +struct rb_ctx {
> +       void *rb;
> +       struct bpf_dynptr dptr;
> +};
> +
> +static __always_inline struct rb_ctx __rb_event_reserve(__u32 sz)
> +{
> +       struct rb_ctx rb_ctx = {};
> +       void *rb;
> +       __u32 cpu = bpf_get_smp_processor_id();
> +       __u32 rb_slot = cpu & 1;
> +
> +       rb = bpf_map_lookup_elem(&rb_in_map, &rb_slot);
> +       if (!rb)
> +               return rb_ctx;
> +
> +       rb_ctx.rb = rb;
> +       bpf_ringbuf_reserve_dynptr(rb, sz, 0, &rb_ctx.dptr);
> +
> +       return rb_ctx;
> +}
> +
> +static __noinline void __rb_event_submit(struct rb_ctx *ctx)
> +{
> +       if (!ctx->rb)
> +               return;
> +
> +       /* If the verifier (incorrectly) concludes that ctx->rb can be
> +        * NULL at this point, we'll get "BPF_EXIT instruction in main
> +        * prog would lead to reference leak" error
> +        */
> +       bpf_ringbuf_submit_dynptr(&ctx->dptr, 0);
> +}
> +
> +SEC("socket")
> +int map_ptr_is_never_null_rb(void *ctx)
> +{
> +       struct rb_ctx event_ctx = __rb_event_reserve(256);
> +       __rb_event_submit(&event_ctx);
> +       return 0;
> +}
> +
>  char _license[] SEC("license") = "GPL";
> --
> 2.47.1
>