On Thu, Nov 9, 2023 at 9:58 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > 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. Right. > Not sure how hard it would be to come up with a selftest for such a scenario. Yep, I'll think of something. Lots of tests to come up with :)
