RE: [PATCH v2 bpf-next 1/5] bpf: Add bpf_dynptr_adjust

[John Fastabend <john.fastabend@gmail.com>]

Joanne Koong wrote:
> Add a new kfunc
> 
> int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end);
> 
> which adjusts the dynptr to reflect the new [start, end) interval.
> In particular, it advances the offset of the dynptr by "start" bytes,
> and if end is less than the size of the dynptr, then this will trim the
> dynptr accordingly.
> 
> Adjusting the dynptr interval may be useful in certain situations.
> For example, when hashing which takes in generic dynptrs, if the dynptr
> points to a struct but only a certain memory region inside the struct
> should be hashed, adjust can be used to narrow in on the
> specific region to hash.

Would you want to prohibit creating an empty dynptr with [start, start)?

> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>  kernel/bpf/helpers.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 00e5fb0682ac..7ddf63ac93ce 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1448,6 +1448,13 @@ u32 bpf_dynptr_get_size(const struct bpf_dynptr_kern *ptr)
>  	return ptr->size & DYNPTR_SIZE_MASK;
>  }
>  
> +static void bpf_dynptr_set_size(struct bpf_dynptr_kern *ptr, u32 new_size)
> +{
> +	u32 metadata = ptr->size & ~DYNPTR_SIZE_MASK;
> +
> +	ptr->size = new_size | metadata;
> +}
> +
>  int bpf_dynptr_check_size(u32 size)
>  {
>  	return size > DYNPTR_MAX_SIZE ? -E2BIG : 0;
> @@ -2297,6 +2304,24 @@ __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 o
>  	return bpf_dynptr_slice(ptr, offset, buffer, buffer__szk);
>  }
>  
> +__bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end)
> +{
> +	u32 size;
> +
> +	if (!ptr->data || start > end)
> +		return -EINVAL;
> +
> +	size = bpf_dynptr_get_size(ptr);
> +
> +	if (start > size || end > size)
> +		return -ERANGE;

maybe 'start >= size'? OTOH if the verifier doesn't mind I guess its OK
to create the thing even if it doesn't make much sense.

> +
> +	ptr->offset += start;
> +	bpf_dynptr_set_size(ptr, end - start);
> +
> +	return 0;
> +}
> +
>  __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
>  {
>  	return obj;
> @@ -2369,6 +2394,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_slice_rdwr, KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_num_new, KF_ITER_NEW)
>  BTF_ID_FLAGS(func, bpf_iter_num_next, KF_ITER_NEXT | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
> +BTF_ID_FLAGS(func, bpf_dynptr_adjust)
>  BTF_SET8_END(common_btf_ids)
>  
>  static const struct btf_kfunc_id_set common_kfunc_set = {
> -- 
> 2.34.1
>