On Thu, Feb 13, 2025 at 08:04:05AM -0800, Alexei Starovoitov wrote: > On Thu, Feb 13, 2025 at 5:13 AM Jiayuan Chen <[email protected]> wrote: > > > > Add test cases to ensure the maximum stack size can be properly limited to > > 512. > > > > Test result: > > echo "0" > /proc/sys/net/core/bpf_jit_enable > > ./test_progs -t verifier_stack_ptr > > verifier_stack_ptr/PTR_TO_STACK stack size 512 with may_goto with jit:SKIP > > verifier_stack_ptr/PTR_TO_STACK stack size 512 with may_goto without jit:OK > > > > echo "1" > /proc/sys/net/core/bpf_jit_enable > > verifier_stack_ptr/PTR_TO_STACK stack size 512 with may_goto with jit:OK > > verifier_stack_ptr/PTR_TO_STACK stack size 512 with may_goto without > > jit:SKIP > > echo '0|1' is not longer necessary ? > The commit log seems obsolete? > > pw-bot: cr
It looks like the problem only arises when CONFIG_BPF_JIT_ALWAYS_ON is turned off, and we're only restricting the stack size when prog->jit_requested is false. To test this, I simulated different scenarios by echoing '0' or '1' to see how the program would behave when jit_requested is enabled or disabled. As expected, when I echoed '0', the program failed verification, and when I echoed '1', it ran smoothly. Thanks > > > Signed-off-by: Jiayuan Chen <[email protected]> > > --- > > .../selftests/bpf/progs/verifier_stack_ptr.c | 50 +++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > > > diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c > > b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c > > index 417c61cd4b19..8ffe5a01d140 100644 > > --- a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c > > +++ b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c > > @@ -481,4 +481,54 @@ l1_%=: r0 = 42; > > \ > > : __clobber_all); > > } > > > > +SEC("socket") > > +__description("PTR_TO_STACK stack size > 512") > > +__failure __msg("invalid write to stack R1 off=-520 size=8") > > +__naked void stack_check_size_gt_512(void) > > +{ > > + asm volatile ( > > + "r1 = r10;" > > + "r1 += -520;" > > + "r0 = 42;" > > + "*(u64*)(r1 + 0) = r0;" > > + "exit;" > > + ::: __clobber_all); > > +} > > + > > +#ifdef __BPF_FEATURE_MAY_GOTO > > +SEC("socket") > > +__description("PTR_TO_STACK stack size 512 with may_goto with jit") > > +__use_jit() > > +__success __retval(42) > > +__naked void stack_check_size_512_with_may_goto_jit(void) > > +{ > > + asm volatile ( > > + "r1 = r10;" > > + "r1 += -512;" > > + "r0 = 42;" > > + "*(u32*)(r1 + 0) = r0;" > > + "may_goto l0_%=;" > > + "r2 = 100;" > > +"l0_%=: exit;" > > + ::: __clobber_all); > > +} > > + > > +SEC("socket") > > +__description("PTR_TO_STACK stack size 512 with may_goto without jit") > > +__use_interp() > > +__failure __msg("stack size 520(extra 8) is too large") > > +__naked void stack_check_size_512_with_may_goto(void) > > +{ > > + asm volatile ( > > + "r1 = r10;" > > + "r1 += -512;" > > + "r0 = 42;" > > + "*(u32*)(r1 + 0) = r0;" > > + "may_goto l0_%=;" > > + "r2 = 100;" > > +"l0_%=: exit;" > > + ::: __clobber_all); > > +} > > +#endif > > + > > char _license[] SEC("license") = "GPL"; > > -- > > 2.47.1 > >
