On Thu, Apr 28, 2022 at 2:12 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> This patch adds a new helper function
>
> void *bpf_dynptr_data(struct bpf_dynptr *ptr, u32 offset, u32 len);
>
> which returns a pointer to the underlying data of a dynptr. *len*
> must be a statically known value. The bpf program may access the returned
> data slice as a normal buffer (eg can do direct reads and writes), since
> the verifier associates the length with the returned pointer, and
> enforces that no out of bounds accesses occur.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> include/linux/bpf.h | 4 +++
> include/uapi/linux/bpf.h | 12 +++++++
> kernel/bpf/helpers.c | 28 +++++++++++++++
> kernel/bpf/verifier.c | 64 ++++++++++++++++++++++++++++++----
> tools/include/uapi/linux/bpf.h | 12 +++++++
> 5 files changed, 114 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b276dbf942dd..4d2de868bdbc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -397,6 +397,9 @@ enum bpf_type_flag {
> /* DYNPTR points to a ringbuf record. */
> DYNPTR_TYPE_RINGBUF = BIT(9 + BPF_BASE_TYPE_BITS),
>
> + /* MEM is memory owned by a dynptr */
> + MEM_DYNPTR = BIT(10 + BPF_BASE_TYPE_BITS),
do we need this yet another bit? It seems like it only matters for
verifier log dynptr_ output? Can we just reuse MEM_ALLOC? Or there is
some ringbuf-specific logic that we'll interfere with? If feels a bit
unnecessary, let's think if we can avoid adding bits just for this.
> +
> __BPF_TYPE_LAST_FLAG = DYNPTR_TYPE_RINGBUF,
> };
>
[...]
> + if (is_dynptr_ref_function(func_id)) {
> + int i;
> +
> + /* Find the id of the dynptr we're acquiring a reference to */
> + for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
> + if (arg_type_is_dynptr(fn->arg_type[i])) {
> + id = stack_slot_get_id(env, ®s[BPF_REG_1 + i]);
let's make sure that we have only one such argument?
> + break;
> + }
> + }
> + if (unlikely(i == MAX_BPF_FUNC_REG_ARGS)) {
please don't use unlikely(), especially for non-performance critical code path
> + verbose(env, "verifier internal error: no dynptr args to a dynptr ref function");
> + return -EFAULT;
> + }
> + } else {
> + id = acquire_reference_state(env, insn_idx);
> + if (id < 0)
> + return id;
> + }
[...]
