On Fri, 1 Nov 2024 at 15:22, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri, Nov 1, 2024 at 1:04 PM Tao Lyu <tao.lyu@xxxxxxx> wrote:
> >
> > Hi,
> >
> > The process_iter_arg check function misses the type check on the iter
> > args, which leads to any pointer types can be passed as iter args.
> >
> > As the attached testcase shows, when I pass a ptr_to_map_value whose
> > offset is 0, process_iter_arg still regards it as a stack pointer and
> > use its offset to check the stack slot types.
> >
> > In this case, as long as the stack slot types matched with the
> > ptr_to_map_value offset is correct, then checks can be bypassed.
> >
> > I attached the fix, which checks if the argument type is stack pointer.
> >
> > Please let me know if this fix might be incomplete.
> > I'm happy to revise it.
> >
> > Best,
> > Tao
> >
> > Signed-off-by: Tao Lyu <tao.lyu@xxxxxxx>
> > ---
> > kernel/bpf/verifier.c | 6 ++++++
> > tools/testing/selftests/bpf/progs/iters.c | 23 +++++++++++++++++++++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 797cf3ed32e0..bc968d2b76d9 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -8031,6 +8031,12 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
> > return -EINVAL;
> > }
> > t = btf_type_by_id(meta->btf, btf_id);
> > +
> > + // Ensure the iter arg is a stack pointer
>
> no c++ comments pls.
>
> Also I believe Kumar sent a fix for this already.
> It fell through the cracks.
>
> Kumar,
> please resend.
For this one, I didn't. I believe the one you're referring to was a
different bug, and yeah, that appears to have fallen through.
I handed it over to someone and they unfortunately happened to switch
jobs after that.
I'll sync with Tao and one of us will resend the other fix.
Tao,
For this, I think you need to fix your subject line as well, and add a
selftest covering this particular case.
Passing in e.g. a map value should be sufficient to test this behavior.
>
> pw-bot: cr
>
> > + if (reg->type != PTR_TO_STACK) {
> > + verbose(env, "iter pointer should be the PTR_TO_STACK type\n");
> > + return -EINVAL;
> > + }
