On Thu, Feb 8, 2024 at 11:40 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Tue, Feb 6, 2024 at 2:04 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > Recognize return of 'void *' from kfunc as returning unknown scalar.
> >
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > ---
> > kernel/bpf/verifier.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index ddaf09db1175..d9c2dbb3939f 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -12353,6 +12353,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> > meta.func_name);
> > return -EFAULT;
> > }
> > + } else if (btf_type_is_void(ptr_type)) {
> > + /* kfunc returning 'void *' is equivalent to returning scalar */
> > + mark_reg_unknown(env, regs, BPF_REG_0);
>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> I think we should do a similar extension when passing `void *` into
> global funcs. It's best to treat it as SCALAR instead of rejecting it
> because we can't calculate the size. Currently users in practice just
> have to define it as `uintptr_t` and then cast (or create static
> wrappers doing the casting). Anyways, my point is that it makes sense
> to treat `void *` as non-pointer.
Makes sense. Will add it to my todo list.
On that note I've been thinking how to get rid of __arg_arena
that I'm adding in this series.
How about the following algorithm?
do_check_main() sees that scalar or ptr_to_arena is passed
into global subprog that has BTF 'struct foo *'
and today would require ptr_to_mem.
Instead of rejecting the prog the verifier would override
(only once and in one direction)
that arg of that global func from ptr_to_mem into scalar.
And will proceed as usual.
do_check_common() of that global subprog will pick up scalar
for that arg, since args are cached.
And verification will proceed successfully without special __arg_arena
.
