Re: [PATCH bpf-next 3/7] bpf: enforce precision for r0 on callback return

Alexei Starovoitov
2023-11-09T17:58:56Z

On Thu, Nov 9, 2023 at 9:50 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
> >
> > The r0 returned from bpf_loop's callback says bpf_loop to stop iteration,
> > bpf_loop returns the number of completed iterations. However, the return
> > value of bpf_loop modeled by verifier is unbounded scalar.
> > Same for map's for each.
>
> return value of bpf_loop() is a different thing from return value of
> bpf_loop's callback. Right now bpf_loop implementation in kernel does
>
> ret = callback(...);
> /* return value: 0 - continue, 1 - stop and return */
> if (ret)
> return i + 1;
>
> So yes, it doesn't rely explicitly on return value to be 1 just due to
> the above implementation. But verifier is meant to enforce that and
> the protocol is that bpf_loop and other callback calling helpers
> should rely on this value.
>
> I think we have the same problem in check_return_code() for entry BPF
> programs. So let me taking this one out of this patch set and post a
> new one concentrating on this particular issue. I've been meaning to
> use umin/umax for return value checking anyways, so might be a good
> idea to do this anyways.

Just like Ed I was also initially confused by this.
As you said check_return_code() has the same problem.
I think the issue this patch and similar in check_return_code()
should be fixing is the case where one state went through
ret code checking, but another state with potentially out-of-range
r0 got state pruned since r0 wasn't marked precise.
Not sure how hard it would be to come up with a selftest for such a scenario.