On Thu, Nov 9, 2023 at 9:50 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> 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.
