On Thu, Feb 8, 2024 at 8:06 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> From: Alexei Starovoitov <ast@xxxxxxxxxx>
>
> In global bpf functions recognize btf_decl_tag("arg:arena") as PTR_TO_ARENA.
>
> Note, when the verifier sees:
>
> __weak void foo(struct bar *p)
>
> it recognizes 'p' as PTR_TO_MEM and 'struct bar' has to be a struct with scalars.
> Hence the only way to use arena pointers in global functions is to tag them with "arg:arena".
>
> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> ---
> include/linux/bpf.h | 1 +
> kernel/bpf/btf.c | 19 +++++++++++++++----
> kernel/bpf/verifier.c | 15 +++++++++++++++
> 3 files changed, 31 insertions(+), 4 deletions(-)
>
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5eeb9bf7e324..fa49602194d5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9348,6 +9348,18 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> bpf_log(log, "arg#%d is expected to be non-NULL\n", i);
> return -EINVAL;
> }
> + } else if (base_type(arg->arg_type) == ARG_PTR_TO_ARENA) {
> + /*
> + * Can pass any value and the kernel won't crash, but
> + * only PTR_TO_ARENA or SCALAR make sense. Everything
> + * else is a bug in the bpf program. Point it out to
> + * the user at the verification time instead of
> + * run-time debug nightmare.
> + */
> + if (reg->type != PTR_TO_ARENA && reg->type != SCALAR_VALUE) {
the comment above doesn't explain why it's ok to pass SCALAR_VALUE. Is
it because PTR_TO_ARENA will become SCALAR_VALUE after some arithmetic
operations and we don't want to regress user experience? If that's the
case, what's the way for user to convert SCALAR_VALUE back to
PTR_TO_ARENA without going through global subprog? bpf_cast_xxx
instruction through assembly?
> + bpf_log(log, "R%d is not a pointer to arena or scalar.\n", regno);
> + return -EINVAL;
> + }
> } else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) {
> ret = process_dynptr_func(env, regno, -1, arg->arg_type, 0);
> if (ret)
> @@ -20329,6 +20341,9 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
> reg->btf = bpf_get_btf_vmlinux(); /* can't fail at this point */
> reg->btf_id = arg->btf_id;
> reg->id = ++env->id_gen;
> + } else if (base_type(arg->arg_type) == ARG_PTR_TO_ARENA) {
> + /* caller can pass either PTR_TO_ARENA or SCALAR */
> + mark_reg_unknown(env, regs, i);
shouldn't we set the register type to PTR_TO_ARENA here?
> } else {
> WARN_ONCE(1, "BUG: unhandled arg#%d type %d\n",
> i - BPF_REG_1, arg->arg_type);
> --
> 2.34.1
>
