On Fri, 2024-07-19 at 19:00 -0700, Alexei Starovoitov wrote: [...] > > It is possible to move remove_nocsr_spills_fills() before > > check_max_stack_depth(), but check_stack_access_within_bounds() would > > still report errors for nocsr stack slots, because > > check_nocsr_stack_contract() and check_stack_access_within_bounds() > > are both invoked during main verification pass and contract validation > > is not yet finished. > > Agree that it's a half measure, but it's still better than doing it > after check_max_stack_depth(). > > We can also allow check_stack_access_within_bounds() to go above 512 > for nocsr pattern. If spill/fill is later removed then great, > if not then it's not a big deal to go slightly above 512 especially > considering that private stack is coming in soon. Ok, I'm going to update check_stack_slot_within_bounds() to allow access for up to 512+48 (to account for all 6 nocsr registers) if accessing instruction is a spill/fill marked as nocsr pattern. It is a speculative thing, because nocsr contract might be found void at some later point during program verification. However check_max_stack_depth_subprog() would still catch access below -512 if nocsr spills are not removed. So, the worst case -- error report for stack usage would be less user friendly. Don't think any other uses of MAX_BPF_STACK need an update, as all the rest seem to deal with registers representing stack pointers.
