Re: [PATCH v4 bpf-next 02/22] bpf: Introduce bpfptr_t user/kernel pointer.

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Fri, May 7, 2021 at 8:48 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Similar to sockptr_t introduce bpfptr_t with few additions:
> make_bpfptr() creates new user/kernel pointer in the same address space as
> existing user/kernel pointer.
> bpfptr_add() advances the user/kernel pointer.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---

LGTM, see minor comment below.

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

>  include/linux/bpfptr.h | 81 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 include/linux/bpfptr.h
>
> diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
> new file mode 100644
> index 000000000000..e370acb04977
> --- /dev/null
> +++ b/include/linux/bpfptr.h
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* A pointer that can point to either kernel or userspace memory. */
> +#ifndef _LINUX_BPFPTR_H
> +#define _LINUX_BPFPTR_H
> +
> +#include <linux/sockptr.h>
> +
> +typedef sockptr_t bpfptr_t;
> +
> +static inline bool bpfptr_is_kernel(bpfptr_t bpfptr)
> +{
> +       return bpfptr.is_kernel;
> +}
> +
> +static inline bpfptr_t KERNEL_BPFPTR(void *p)
> +{
> +       return (bpfptr_t) { .kernel = p, .is_kernel = true };
> +}
> +
> +static inline bpfptr_t USER_BPFPTR(void __user *p)
> +{
> +       return (bpfptr_t) { .user = p };
> +}
> +
> +static inline bpfptr_t make_bpfptr(u64 addr, bool is_kernel)
> +{
> +       if (is_kernel)
> +               return (bpfptr_t) {
> +                       .kernel = (void*) (uintptr_t) addr,
> +                       .is_kernel = true,
> +               };
> +       else
> +               return (bpfptr_t) {
> +                       .user = u64_to_user_ptr(addr),
> +                       .is_kernel = false,
> +               };

Given there are KERNEL_BPFPTR and USER_BPFPTR constructors, any reason
to not use them here?

> +}
> +

[...]