On Mon, Feb 5, 2024 at 10:30 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote: > > With latest llvm19, I hit the following selftest failures with > > $ ./test_progs -j > libbpf: prog 'on_event': BPF program load failed: Permission denied > libbpf: prog 'on_event': -- BEGIN PROG LOAD LOG -- > combined stack size of 4 calls is 544. Too large > verification time 1344153 usec > stack depth 24+440+0+32 > processed 51008 insns (limit 1000000) max_states_per_insn 19 total_states 1467 peak_states 303 mark_read 146 > -- END PROG LOAD LOG -- > libbpf: prog 'on_event': failed to load: -13 > libbpf: failed to load object 'strobemeta_subprogs.bpf.o' > scale_test:FAIL:expect_success unexpected error: -13 (errno 13) > #498 verif_scale_strobemeta_subprogs:FAIL > > The verifier complains too big of the combined stack size (544 bytes) which > exceeds the maximum stack limit 512. This is a regression from llvm19 ([1]). > > In the above error log, the original stack depth is 24+440+0+32. > To satisfy interpreter's need, in verifier the stack depth is adjusted to > 32+448+32+32=544 which exceeds 512, hence the error. The same adjusted > stack size is also used for jit case. > > But the jitted codes could use smaller stack size. > > $ egrep -r stack_depth | grep round_up > arm64/net/bpf_jit_comp.c: ctx->stack_size = round_up(prog->aux->stack_depth, 16); > loongarch/net/bpf_jit.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16); > powerpc/net/bpf_jit_comp.c: cgctx.stack_size = round_up(fp->aux->stack_depth, 16); > riscv/net/bpf_jit_comp32.c: round_up(ctx->prog->aux->stack_depth, STACK_ALIGN); > riscv/net/bpf_jit_comp64.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16); > s390/net/bpf_jit_comp.c: u32 stack_depth = round_up(fp->aux->stack_depth, 8); > sparc/net/bpf_jit_comp_64.c: stack_needed += round_up(stack_depth, 16); > x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xEC, round_up(stack_depth, 8)); > x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8); > x86/net/bpf_jit_comp.c: round_up(stack_depth, 8)); > x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8); > x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xC4, round_up(stack_depth, 8)); > > In the above, STACK_ALIGN in riscv/net/bpf_jit_comp32.c is defined as 16. > So stack is aligned in either 8 or 16, x86/s390 having 8-byte stack alignment and > the rest having 16-byte alignment. > > This patch calculates total stack depth based on 16-byte alignment if jit is requested. > For the above failing case, the new stack size will be 32+448+0+32=512 and no verification > failure. llvm19 regression will be discussed separately in llvm upstream. > > [1] https://lore.kernel.org/bpf/32bde0f0-1881-46c9-931a-673be566c61d@xxxxxxxxx/ > > Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx> > Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx> > --- Seems like a few selftests have to be adjusted, current BPF CI is unhappy ([0]) [0] https://github.com/kernel-patches/bpf/actions/runs/7795686155/job/21259132248 pw-bot: cr > kernel/bpf/verifier.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index ddaf09db1175..10e33d49ca21 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5812,6 +5812,17 @@ static int check_ptr_alignment(struct bpf_verifier_env *env, > strict); > } > > +static int round_up_stack_depth(struct bpf_verifier_env *env, int stack_depth) > +{ > + if (env->prog->jit_requested) > + return round_up(stack_depth, 16); > + > + /* round up to 32-bytes, since this is granularity > + * of interpreter stack size > + */ > + return round_up(max_t(u32, stack_depth, 1), 32); > +} > + > /* starting from main bpf function walk all instructions of the function > * and recursively walk all callees that given function can call. > * Ignore jump and exit insns. > @@ -5855,10 +5866,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx) > depth); > return -EACCES; > } > - /* round up to 32-bytes, since this is granularity > - * of interpreter stack size > - */ > - depth += round_up(max_t(u32, subprog[idx].stack_depth, 1), 32); > + > + depth += round_up_stack_depth(env, subprog[idx].stack_depth); > if (depth > MAX_BPF_STACK) { > verbose(env, "combined stack size of %d calls is %d. Too large\n", > frame + 1, depth); > @@ -5952,7 +5961,7 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx) > */ > if (frame == 0) > return 0; > - depth -= round_up(max_t(u32, subprog[idx].stack_depth, 1), 32); > + depth -= round_up_stack_depth(env, subprog[idx].stack_depth); > frame--; > i = ret_insn[frame]; > idx = ret_prog[frame]; > -- > 2.34.1 >
