On Fri, Apr 1, 2022 at 7:00 PM Joanne Koong <joannekoong@xxxxxx> wrote:
>
> From: Joanne Koong <joannelkoong@xxxxxxxxx>
>
> Currently, we hardcode in the verifier which functions are release
> functions. We have no way of differentiating which argument is the one
> to be released (we assume it will always be the first argument).
>
> This patch adds MEM_RELEASE as a bpf_type_flag. This allows us to
> determine which argument in the function needs to be released, and
> removes having to hardcode a list of release functions into the
> verifier.
>
> Please note that currently, we only support one release argument in a
> helper function. In the future, if/when we need to support several
> release arguments within the function, MEM_RELEASE is necessary
> since there needs to be a way of differentiating which arguments are the
> release ones.
>
> In the near future, MEM_RELEASE will be used by dynptr helper functions
> such as bpf_free.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> include/linux/bpf.h | 4 +++-
> include/linux/bpf_verifier.h | 3 +--
> kernel/bpf/btf.c | 3 ++-
> kernel/bpf/ringbuf.c | 4 ++--
> kernel/bpf/verifier.c | 42 ++++++++++++++++++------------------
> net/core/filter.c | 2 +-
> 6 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 6f2558da9d4a..cb9f42866cde 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -344,7 +344,9 @@ enum bpf_type_flag {
>
> MEM_UNINIT = BIT(5 + BPF_BASE_TYPE_BITS),
>
> - __BPF_TYPE_LAST_FLAG = MEM_UNINIT,
> + MEM_RELEASE = BIT(6 + BPF_BASE_TYPE_BITS),
"MEM_" part seems a bit too specific, it's not necessarily (just)
about memory, it's more generally about "releasing resources" in
general, right? ARG_RELEASE or OBJ_RELEASE maybe?
> +
> + __BPF_TYPE_LAST_FLAG = MEM_RELEASE,
> };
>
[...]
> -/* Determine whether the function releases some resources allocated by another
> - * function call. The first reference type argument will be assumed to be
> - * released by release_reference().
> +/* Determine whether the type releases some resources allocated by a
> + * previous function call.
> */
> -static bool is_release_function(enum bpf_func_id func_id)
> +static bool type_is_release_mem(u32 type)
> {
> - return func_id == BPF_FUNC_sk_release ||
> - func_id == BPF_FUNC_ringbuf_submit ||
> - func_id == BPF_FUNC_ringbuf_discard;
> + return type & MEM_RELEASE;
> }
>
same skepticism regarding the need for this helper function, just
makes grepping code slightly harder
> static bool may_be_acquire_function(enum bpf_func_id func_id)
[...]
