On Wed, 19 Feb 2025 at 13:51, Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > For the bpf_dynptr_slice_rdwr kfunc, the verifier may return a pointer > to the underlying packet (if the requested slice is linear), or copy out > the data to the buffer passed into the kfunc. The verifier performs > symbolic execution assuming the returned value is a PTR_TO_MEM of a > certain size (passed into the kfunc), and ensures reads and writes are > within bounds. > > A complication arises when the passed in buffer is a stack pointer. The > returned pointer may be used to perform writes (unlike bpf_dynptr_slice), > but the verifier will be unaware of which stack slots such writes may > end up overwriting. As such, a program may end up overwriting stack data > (such as spilled pointers) through such return values by accident, which > may cause undefined behavior. > > Fix this by exploring an additional path whenever the passed in argument > is a PTR_TO_STACK, and explore a path where the returned buffer is the > same as this stack pointer. This allows us to ensure that the program > doesn't introduce unsafe behavior when this condition is triggered at > runtime. > > The push_stack() call is performed after kfunc processing is over, > simply fixing up the return type to PTR_TO_STACK with proper frameno, > off, and var_off. I just sent this out to have discussion with code and context (in selftest). The current approach has two problems: It fails xdp_dynptr selftest with misaligned stack access (-72+30 = 42) size 4 error. This was happening before as well, but is surfaced because we see writes to the stack. It also leads to veristat regression (+80-100% in states) in two selftests. We probably want to avoid doing push_stack due to the states increase, and instead mark the stack slot instead whenever the returned PTR_TO_MEM is used for writing, but we'll have to keep remarking whenever writes happen, so it requires stashing some stack slot state in the register. The other option is invalidating the returned PTR_TO_MEM when the buffer on the stack is written to (i.e. the stack location gets reused).
