Re: [PATCH v5 bpf-next 1/4] bpf: Allow ringbuf memory to be used as map key

Andrii Nakryiko
2022-10-21T23:04:38Z

On Thu, Oct 20, 2022 at 9:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> This patch adds support for the following pattern:
>
> struct some_data *data = bpf_ringbuf_reserve(&ringbuf, sizeof(struct some_data, 0));
> if (!data)
> return;
> bpf_map_lookup_elem(&another_map, &data->some_field);
> bpf_ringbuf_submit(data);
>
> Currently the verifier does not consider bpf_ringbuf_reserve's
> PTR_TO_MEM | MEM_ALLOC ret type a valid key input to bpf_map_lookup_elem.
> Since PTR_TO_MEM is by definition a valid region of memory, it is safe
> to use it as a key for lookups.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> ---

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> v2->v3: lore.kernel.org/bpf/20220914123600.927632-1-davemarchevsky@fb.com
>
> * Add Yonghong ack, rebase
>
> v1->v2: lore.kernel.org/bpf/20220912101106.2765921-1-davemarchevsky@fb.com
>
> * Move test changes into separate patch - patch 2 in this series.
> (Kumar, Yonghong). That patch's changelog enumerates specific
> changes from v1
> * Remove PTR_TO_MEM addition from this patch - patch 1 (Yonghong)
> * I don't have a usecase for PTR_TO_MEM w/o MEM_ALLOC
> * Add "if (!data)" error check to example pattern in this patch
> (Yonghong)
> * Remove patch 2 from v1's series, which removed map_key_value_types
> as it was more-or-less duplicate of mem_types
> * Now that PTR_TO_MEM isn't added here, more differences between
> map_key_value_types and mem_types, and no usecase for PTR_TO_BUF,
> so drop for now.
>
> kernel/bpf/verifier.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6f6d2d511c06..97351ae3e7a7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5641,6 +5641,7 @@ static const struct bpf_reg_types map_key_value_types = {
> PTR_TO_PACKET_META,
> PTR_TO_MAP_KEY,
> PTR_TO_MAP_VALUE,
> + PTR_TO_MEM | MEM_ALLOC,
> },
> };
>
> --
> 2.30.2
>