* [PATCH bpf-next v2 0/3] bpf: Reduce verifier stack frame size
@ 2025-07-02 17:11 Yonghong Song
2025-07-02 17:11 ` [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() Yonghong Song
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yonghong Song @ 2025-07-02 17:11 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Arnd Bergmann reported an issue ([1]) where clang compiler (less than
llvm18) may trigger an error where the stack frame size exceeds the limit.
I can reproduce the error like below:
kernel/bpf/verifier.c:24491:5: error: stack frame size (2552) exceeds limit (1280) in 'bpf_check'
[-Werror,-Wframe-larger-than]
kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check'
[-Werror,-Wframe-larger-than]
This patch series fixed the above two errors by reducing stack size.
See each individual patches for details.
[1] https://lore.kernel.org/bpf/20250620113846.3950478-1-arnd@kernel.org/
Changelogs:
v1 -> v2:
- v1: https://lore.kernel.org/bpf/20250702053332.1991516-1-yonghong.song@linux.dev/
- Simplify assignment to struct bpf_insn pointer in do_misc_fixups().
- Restore original implementation in opt_hard_wire_dead_code_branches()
as only one insn on the stack.
- Avoid unnecessary insns for 64bit modulo (mod 0/-1) operations.
Yonghong Song (3):
bpf: Simplify assignment to struct bpf_insn pointer in
do_misc_fixups()
bpf: Reduce stack frame size by using env->insn_buf for bpf insns
bpf: Avoid putting struct bpf_scc_callchain variables on the stack
include/linux/bpf_verifier.h | 1 +
kernel/bpf/verifier.c | 229 +++++++++++++++++------------------
2 files changed, 112 insertions(+), 118 deletions(-)
--
2.47.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() 2025-07-02 17:11 [PATCH bpf-next v2 0/3] bpf: Reduce verifier stack frame size Yonghong Song @ 2025-07-02 17:11 ` Yonghong Song 2025-07-02 18:58 ` Eduard Zingerman 2025-07-02 17:11 ` [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack Yonghong Song 2 siblings, 1 reply; 9+ messages in thread From: Yonghong Song @ 2025-07-02 17:11 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau In verifier.c, the following code patterns (in two places) struct bpf_insn *patch = &insn_buf[0]; can be simplified to struct bpf_insn *patch = insn_buf; which is easier to understand. Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- kernel/bpf/verifier.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 52e36fd23f40..8b0a25851089 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -22102,7 +22102,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env) if (BPF_CLASS(insn->code) == BPF_LDX && (BPF_MODE(insn->code) == BPF_PROBE_MEM || BPF_MODE(insn->code) == BPF_PROBE_MEMSX)) { - struct bpf_insn *patch = &insn_buf[0]; + struct bpf_insn *patch = insn_buf; u64 uaddress_limit = bpf_arch_uaddress_limit(); if (!uaddress_limit) @@ -22153,7 +22153,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env) insn->code == (BPF_ALU64 | BPF_SUB | BPF_X)) { const u8 code_add = BPF_ALU64 | BPF_ADD | BPF_X; const u8 code_sub = BPF_ALU64 | BPF_SUB | BPF_X; - struct bpf_insn *patch = &insn_buf[0]; + struct bpf_insn *patch = insn_buf; bool issrc, isneg, isimm; u32 off_reg; -- 2.47.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() 2025-07-02 17:11 ` [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() Yonghong Song @ 2025-07-02 18:58 ` Eduard Zingerman 0 siblings, 0 replies; 9+ messages in thread From: Eduard Zingerman @ 2025-07-02 18:58 UTC (permalink / raw) To: Yonghong Song, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau On Wed, 2025-07-02 at 10:11 -0700, Yonghong Song wrote: > In verifier.c, the following code patterns (in two places) > struct bpf_insn *patch = &insn_buf[0]; > can be simplified to > struct bpf_insn *patch = insn_buf; > which is easier to understand. > > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns 2025-07-02 17:11 [PATCH bpf-next v2 0/3] bpf: Reduce verifier stack frame size Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() Yonghong Song @ 2025-07-02 17:11 ` Yonghong Song 2025-07-02 20:33 ` Eduard Zingerman 2025-07-02 17:11 ` [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack Yonghong Song 2 siblings, 1 reply; 9+ messages in thread From: Yonghong Song @ 2025-07-02 17:11 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Arnd Bergmann, Jiri Olsa Arnd Bergmann reported an issue ([1]) where clang compiler (less than llvm18) may trigger an error where the stack frame size exceeds the limit. I can reproduce the error like below: kernel/bpf/verifier.c:24491:5: error: stack frame size (2552) exceeds limit (1280) in 'bpf_check' [-Werror,-Wframe-larger-than] kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' [-Werror,-Wframe-larger-than] Use env->insn_buf for bpf insns instead of putting these insns on the stack. This can resolve the above 'bpf_check' error. The 'do_check' error will be resolved in the next patch. [1] https://lore.kernel.org/bpf/20250620113846.3950478-1-arnd@kernel.org/ Reported-by: Arnd Bergmann <arnd@kernel.org> Tested-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- kernel/bpf/verifier.c | 189 ++++++++++++++++++++---------------------- 1 file changed, 91 insertions(+), 98 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8b0a25851089..ef53e313d841 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -21010,7 +21010,9 @@ static int opt_remove_nops(struct bpf_verifier_env *env) static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, const union bpf_attr *attr) { - struct bpf_insn *patch, zext_patch[2], rnd_hi32_patch[4]; + struct bpf_insn *patch; + struct bpf_insn *zext_patch = env->insn_buf; + struct bpf_insn *rnd_hi32_patch = &env->insn_buf[2]; struct bpf_insn_aux_data *aux = env->insn_aux_data; int i, patch_len, delta = 0, len = env->prog->len; struct bpf_insn *insns = env->prog->insnsi; @@ -21188,13 +21190,12 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) if (env->insn_aux_data[i + delta].nospec) { WARN_ON_ONCE(env->insn_aux_data[i + delta].alu_state); - struct bpf_insn patch[] = { - BPF_ST_NOSPEC(), - *insn, - }; + struct bpf_insn *patch = insn_buf; - cnt = ARRAY_SIZE(patch); - new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt); + *patch++ = BPF_ST_NOSPEC(); + *patch++ = *insn; + cnt = patch - insn_buf; + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); if (!new_prog) return -ENOMEM; @@ -21262,13 +21263,12 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) /* nospec_result is only used to mitigate Spectre v4 and * to limit verification-time for Spectre v1. */ - struct bpf_insn patch[] = { - *insn, - BPF_ST_NOSPEC(), - }; + struct bpf_insn *patch = insn_buf; - cnt = ARRAY_SIZE(patch); - new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt); + *patch++ = *insn; + *patch++ = BPF_ST_NOSPEC(); + cnt = patch - insn_buf; + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); if (!new_prog) return -ENOMEM; @@ -21938,13 +21938,12 @@ static int do_misc_fixups(struct bpf_verifier_env *env) u16 stack_depth_extra = 0; if (env->seen_exception && !env->exception_callback_subprog) { - struct bpf_insn patch[] = { - env->prog->insnsi[insn_cnt - 1], - BPF_MOV64_REG(BPF_REG_0, BPF_REG_1), - BPF_EXIT_INSN(), - }; + struct bpf_insn *patch = insn_buf; - ret = add_hidden_subprog(env, patch, ARRAY_SIZE(patch)); + *patch++ = env->prog->insnsi[insn_cnt - 1]; + *patch++ = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1); + *patch++ = BPF_EXIT_INSN(); + ret = add_hidden_subprog(env, insn_buf, patch - insn_buf); if (ret < 0) return ret; prog = env->prog; @@ -21980,20 +21979,18 @@ static int do_misc_fixups(struct bpf_verifier_env *env) insn->off == 1 && insn->imm == -1) { bool is64 = BPF_CLASS(insn->code) == BPF_ALU64; bool isdiv = BPF_OP(insn->code) == BPF_DIV; - struct bpf_insn *patchlet; - struct bpf_insn chk_and_sdiv[] = { - BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | - BPF_NEG | BPF_K, insn->dst_reg, - 0, 0, 0), - }; - struct bpf_insn chk_and_smod[] = { - BPF_MOV32_IMM(insn->dst_reg, 0), - }; + struct bpf_insn *patch = insn_buf; - patchlet = isdiv ? chk_and_sdiv : chk_and_smod; - cnt = isdiv ? ARRAY_SIZE(chk_and_sdiv) : ARRAY_SIZE(chk_and_smod); + if (isdiv) + *patch++ = BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | + BPF_NEG | BPF_K, insn->dst_reg, + 0, 0, 0); + else + *patch++ = BPF_MOV32_IMM(insn->dst_reg, 0); + + cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); if (!new_prog) return -ENOMEM; @@ -22012,83 +22009,79 @@ static int do_misc_fixups(struct bpf_verifier_env *env) bool isdiv = BPF_OP(insn->code) == BPF_DIV; bool is_sdiv = isdiv && insn->off == 1; bool is_smod = !isdiv && insn->off == 1; - struct bpf_insn *patchlet; - struct bpf_insn chk_and_div[] = { - /* [R,W]x div 0 -> 0 */ - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JNE | BPF_K, insn->src_reg, - 0, 2, 0), - BPF_ALU32_REG(BPF_XOR, insn->dst_reg, insn->dst_reg), - BPF_JMP_IMM(BPF_JA, 0, 0, 1), - *insn, - }; - struct bpf_insn chk_and_mod[] = { - /* [R,W]x mod 0 -> [R,W]x */ - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JEQ | BPF_K, insn->src_reg, - 0, 1 + (is64 ? 0 : 1), 0), - *insn, - BPF_JMP_IMM(BPF_JA, 0, 0, 1), - BPF_MOV32_REG(insn->dst_reg, insn->dst_reg), - }; - struct bpf_insn chk_and_sdiv[] = { + struct bpf_insn *patch = insn_buf; + + if (is_sdiv) { /* [R,W]x sdiv 0 -> 0 * LLONG_MIN sdiv -1 -> LLONG_MIN * INT_MIN sdiv -1 -> INT_MIN */ - BPF_MOV64_REG(BPF_REG_AX, insn->src_reg), - BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | - BPF_ADD | BPF_K, BPF_REG_AX, - 0, 0, 1), - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JGT | BPF_K, BPF_REG_AX, - 0, 4, 1), - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JEQ | BPF_K, BPF_REG_AX, - 0, 1, 0), - BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | - BPF_MOV | BPF_K, insn->dst_reg, - 0, 0, 0), + *patch++ = BPF_MOV64_REG(BPF_REG_AX, insn->src_reg); + *patch++ = BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | + BPF_ADD | BPF_K, BPF_REG_AX, + 0, 0, 1); + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JGT | BPF_K, BPF_REG_AX, + 0, 4, 1); + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JEQ | BPF_K, BPF_REG_AX, + 0, 1, 0); + *patch++ = BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | + BPF_MOV | BPF_K, insn->dst_reg, + 0, 0, 0); /* BPF_NEG(LLONG_MIN) == -LLONG_MIN == LLONG_MIN */ - BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | - BPF_NEG | BPF_K, insn->dst_reg, - 0, 0, 0), - BPF_JMP_IMM(BPF_JA, 0, 0, 1), - *insn, - }; - struct bpf_insn chk_and_smod[] = { + *patch++ = BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | + BPF_NEG | BPF_K, insn->dst_reg, + 0, 0, 0); + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); + *patch++ = *insn; + cnt = patch - insn_buf; + } else if (is_smod) { /* [R,W]x mod 0 -> [R,W]x */ /* [R,W]x mod -1 -> 0 */ - BPF_MOV64_REG(BPF_REG_AX, insn->src_reg), - BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | - BPF_ADD | BPF_K, BPF_REG_AX, - 0, 0, 1), - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JGT | BPF_K, BPF_REG_AX, - 0, 3, 1), - BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | - BPF_JEQ | BPF_K, BPF_REG_AX, - 0, 3 + (is64 ? 0 : 1), 1), - BPF_MOV32_IMM(insn->dst_reg, 0), - BPF_JMP_IMM(BPF_JA, 0, 0, 1), - *insn, - BPF_JMP_IMM(BPF_JA, 0, 0, 1), - BPF_MOV32_REG(insn->dst_reg, insn->dst_reg), - }; - - if (is_sdiv) { - patchlet = chk_and_sdiv; - cnt = ARRAY_SIZE(chk_and_sdiv); - } else if (is_smod) { - patchlet = chk_and_smod; - cnt = ARRAY_SIZE(chk_and_smod) - (is64 ? 2 : 0); + *patch++ = BPF_MOV64_REG(BPF_REG_AX, insn->src_reg); + *patch++ = BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) | + BPF_ADD | BPF_K, BPF_REG_AX, + 0, 0, 1); + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JGT | BPF_K, BPF_REG_AX, + 0, 3, 1); + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JEQ | BPF_K, BPF_REG_AX, + 0, 3 + (is64 ? 0 : 1), 1); + *patch++ = BPF_MOV32_IMM(insn->dst_reg, 0); + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); + *patch++ = *insn; + + if (!is64) { + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); + *patch++ = BPF_MOV32_REG(insn->dst_reg, insn->dst_reg); + } + cnt = patch - insn_buf; + } else if (isdiv) { + /* [R,W]x div 0 -> 0 */ + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JNE | BPF_K, insn->src_reg, + 0, 2, 0); + *patch++ = BPF_ALU32_REG(BPF_XOR, insn->dst_reg, insn->dst_reg); + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); + *patch++ = *insn; + cnt = patch - insn_buf; } else { - patchlet = isdiv ? chk_and_div : chk_and_mod; - cnt = isdiv ? ARRAY_SIZE(chk_and_div) : - ARRAY_SIZE(chk_and_mod) - (is64 ? 2 : 0); + /* [R,W]x mod 0 -> [R,W]x */ + *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | + BPF_JEQ | BPF_K, insn->src_reg, + 0, 1 + (is64 ? 0 : 1), 0); + *patch++ = *insn; + + if (!is64) { + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); + *patch++ = BPF_MOV32_REG(insn->dst_reg, insn->dst_reg); + } + cnt = patch - insn_buf; } - new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); if (!new_prog) return -ENOMEM; -- 2.47.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns 2025-07-02 17:11 ` [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Yonghong Song @ 2025-07-02 20:33 ` Eduard Zingerman 2025-07-03 2:02 ` Yonghong Song 0 siblings, 1 reply; 9+ messages in thread From: Eduard Zingerman @ 2025-07-02 20:33 UTC (permalink / raw) To: Yonghong Song, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Arnd Bergmann, Jiri Olsa On Wed, 2025-07-02 at 10:11 -0700, Yonghong Song wrote: > Arnd Bergmann reported an issue ([1]) where clang compiler (less than > llvm18) may trigger an error where the stack frame size exceeds the limit. > I can reproduce the error like below: > kernel/bpf/verifier.c:24491:5: error: stack frame size (2552) exceeds limit (1280) in 'bpf_check' > [-Werror,-Wframe-larger-than] > kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' > [-Werror,-Wframe-larger-than] > > Use env->insn_buf for bpf insns instead of putting these insns on the > stack. This can resolve the above 'bpf_check' error. The 'do_check' error > will be resolved in the next patch. > > [1] https://lore.kernel.org/bpf/20250620113846.3950478-1-arnd@kernel.org/ > > Reported-by: Arnd Bergmann <arnd@kernel.org> > Tested-by: Arnd Bergmann <arnd@arndb.de> > Acked-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> > kernel/bpf/verifier.c | 189 ++++++++++++++++++++---------------------- > 1 file changed, 91 insertions(+), 98 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 8b0a25851089..ef53e313d841 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -21010,7 +21010,9 @@ static int opt_remove_nops(struct bpf_verifier_env *env) > static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, > const union bpf_attr *attr) > { > - struct bpf_insn *patch, zext_patch[2], rnd_hi32_patch[4]; > + struct bpf_insn *patch; > + struct bpf_insn *zext_patch = env->insn_buf; > + struct bpf_insn *rnd_hi32_patch = &env->insn_buf[2]; Nit: I'd add a comment here, something along the lines: "use env->insn_buf as two independent buffers" > struct bpf_insn_aux_data *aux = env->insn_aux_data; > int i, patch_len, delta = 0, len = env->prog->len; > struct bpf_insn *insns = env->prog->insnsi; [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns 2025-07-02 20:33 ` Eduard Zingerman @ 2025-07-03 2:02 ` Yonghong Song 0 siblings, 0 replies; 9+ messages in thread From: Yonghong Song @ 2025-07-03 2:02 UTC (permalink / raw) To: Eduard Zingerman, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Arnd Bergmann, Jiri Olsa On 7/2/25 1:33 PM, Eduard Zingerman wrote: > On Wed, 2025-07-02 at 10:11 -0700, Yonghong Song wrote: >> Arnd Bergmann reported an issue ([1]) where clang compiler (less than >> llvm18) may trigger an error where the stack frame size exceeds the limit. >> I can reproduce the error like below: >> kernel/bpf/verifier.c:24491:5: error: stack frame size (2552) exceeds limit (1280) in 'bpf_check' >> [-Werror,-Wframe-larger-than] >> kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' >> [-Werror,-Wframe-larger-than] >> >> Use env->insn_buf for bpf insns instead of putting these insns on the >> stack. This can resolve the above 'bpf_check' error. The 'do_check' error >> will be resolved in the next patch. >> >> [1] https://lore.kernel.org/bpf/20250620113846.3950478-1-arnd@kernel.org/ >> >> Reported-by: Arnd Bergmann <arnd@kernel.org> >> Tested-by: Arnd Bergmann <arnd@arndb.de> >> Acked-by: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >> --- > Acked-by: Eduard Zingerman <eddyz87@gmail.com> > >> kernel/bpf/verifier.c | 189 ++++++++++++++++++++---------------------- >> 1 file changed, 91 insertions(+), 98 deletions(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 8b0a25851089..ef53e313d841 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -21010,7 +21010,9 @@ static int opt_remove_nops(struct bpf_verifier_env *env) >> static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, >> const union bpf_attr *attr) >> { >> - struct bpf_insn *patch, zext_patch[2], rnd_hi32_patch[4]; >> + struct bpf_insn *patch; >> + struct bpf_insn *zext_patch = env->insn_buf; >> + struct bpf_insn *rnd_hi32_patch = &env->insn_buf[2]; > Nit: I'd add a comment here, something along the lines: > "use env->insn_buf as two independent buffers" Ack. Sounds a good idea. > >> struct bpf_insn_aux_data *aux = env->insn_aux_data; >> int i, patch_len, delta = 0, len = env->prog->len; >> struct bpf_insn *insns = env->prog->insnsi; > [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack 2025-07-02 17:11 [PATCH bpf-next v2 0/3] bpf: Reduce verifier stack frame size Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Yonghong Song @ 2025-07-02 17:11 ` Yonghong Song 2025-07-02 19:03 ` Eduard Zingerman 2 siblings, 1 reply; 9+ messages in thread From: Yonghong Song @ 2025-07-02 17:11 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Jiri Olsa Add a 'struct bpf_scc_callchain callchain' field in bpf_verifier_env. This way, the previous bpf_scc_callchain local variables can be replaced by taking address of env->callchain. This can reduce stack usage and fix the following error: kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' [-Werror,-Wframe-larger-than] Reported-by: Arnd Bergmann <arnd@kernel.org> Acked-by: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- include/linux/bpf_verifier.h | 1 + kernel/bpf/verifier.c | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7e459e839f8b..e2c175d608bb 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -841,6 +841,7 @@ struct bpf_verifier_env { char tmp_str_buf[TMP_STR_BUF_LEN]; struct bpf_insn insn_buf[INSN_BUF_SIZE]; struct bpf_insn epilogue_buf[INSN_BUF_SIZE]; + struct bpf_scc_callchain callchain; /* array of pointers to bpf_scc_info indexed by SCC id */ struct bpf_scc_info **scc_info; u32 scc_cnt; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ef53e313d841..3bf1f58c81a3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1913,19 +1913,19 @@ static char *format_callchain(struct bpf_verifier_env *env, struct bpf_scc_callc */ static int maybe_enter_scc(struct bpf_verifier_env *env, struct bpf_verifier_state *st) { - struct bpf_scc_callchain callchain; + struct bpf_scc_callchain *callchain = &env->callchain; struct bpf_scc_visit *visit; - if (!compute_scc_callchain(env, st, &callchain)) + if (!compute_scc_callchain(env, st, callchain)) return 0; - visit = scc_visit_lookup(env, &callchain); - visit = visit ?: scc_visit_alloc(env, &callchain); + visit = scc_visit_lookup(env, callchain); + visit = visit ?: scc_visit_alloc(env, callchain); if (!visit) return -ENOMEM; if (!visit->entry_state) { visit->entry_state = st; if (env->log.level & BPF_LOG_LEVEL2) - verbose(env, "SCC enter %s\n", format_callchain(env, &callchain)); + verbose(env, "SCC enter %s\n", format_callchain(env, callchain)); } return 0; } @@ -1938,21 +1938,21 @@ static int propagate_backedges(struct bpf_verifier_env *env, struct bpf_scc_visi */ static int maybe_exit_scc(struct bpf_verifier_env *env, struct bpf_verifier_state *st) { - struct bpf_scc_callchain callchain; + struct bpf_scc_callchain *callchain = &env->callchain; struct bpf_scc_visit *visit; - if (!compute_scc_callchain(env, st, &callchain)) + if (!compute_scc_callchain(env, st, callchain)) return 0; - visit = scc_visit_lookup(env, &callchain); + visit = scc_visit_lookup(env, callchain); if (!visit) { verifier_bug(env, "scc exit: no visit info for call chain %s", - format_callchain(env, &callchain)); + format_callchain(env, callchain)); return -EFAULT; } if (visit->entry_state != st) return 0; if (env->log.level & BPF_LOG_LEVEL2) - verbose(env, "SCC exit %s\n", format_callchain(env, &callchain)); + verbose(env, "SCC exit %s\n", format_callchain(env, callchain)); visit->entry_state = NULL; env->num_backedges -= visit->num_backedges; visit->num_backedges = 0; @@ -1967,22 +1967,22 @@ static int add_scc_backedge(struct bpf_verifier_env *env, struct bpf_verifier_state *st, struct bpf_scc_backedge *backedge) { - struct bpf_scc_callchain callchain; + struct bpf_scc_callchain *callchain = &env->callchain; struct bpf_scc_visit *visit; - if (!compute_scc_callchain(env, st, &callchain)) { + if (!compute_scc_callchain(env, st, callchain)) { verifier_bug(env, "add backedge: no SCC in verification path, insn_idx %d", st->insn_idx); return -EFAULT; } - visit = scc_visit_lookup(env, &callchain); + visit = scc_visit_lookup(env, callchain); if (!visit) { verifier_bug(env, "add backedge: no visit info for call chain %s", - format_callchain(env, &callchain)); + format_callchain(env, callchain)); return -EFAULT; } if (env->log.level & BPF_LOG_LEVEL2) - verbose(env, "SCC backedge %s\n", format_callchain(env, &callchain)); + verbose(env, "SCC backedge %s\n", format_callchain(env, callchain)); backedge->next = visit->backedges; visit->backedges = backedge; visit->num_backedges++; @@ -1998,12 +1998,12 @@ static int add_scc_backedge(struct bpf_verifier_env *env, static bool incomplete_read_marks(struct bpf_verifier_env *env, struct bpf_verifier_state *st) { - struct bpf_scc_callchain callchain; + struct bpf_scc_callchain *callchain = &env->callchain; struct bpf_scc_visit *visit; - if (!compute_scc_callchain(env, st, &callchain)) + if (!compute_scc_callchain(env, st, callchain)) return false; - visit = scc_visit_lookup(env, &callchain); + visit = scc_visit_lookup(env, callchain); if (!visit) return false; return !!visit->backedges; -- 2.47.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack 2025-07-02 17:11 ` [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack Yonghong Song @ 2025-07-02 19:03 ` Eduard Zingerman 2025-07-03 2:01 ` Yonghong Song 0 siblings, 1 reply; 9+ messages in thread From: Eduard Zingerman @ 2025-07-02 19:03 UTC (permalink / raw) To: Yonghong Song, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Jiri Olsa On Wed, 2025-07-02 at 10:11 -0700, Yonghong Song wrote: > Add a 'struct bpf_scc_callchain callchain' field in bpf_verifier_env. > This way, the previous bpf_scc_callchain local variables can be > replaced by taking address of env->callchain. This can reduce stack > usage and fix the following error: > kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' > [-Werror,-Wframe-larger-than] > > Reported-by: Arnd Bergmann <arnd@kernel.org> > Acked-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- Oh, well. I liked stack allocation for callchain object, because it emphasized its ephemeral by-value status. The changes lgtm, all places with callchain stack allocation replaced. Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...] > include/linux/bpf_verifier.h | 1 + > kernel/bpf/verifier.c | 36 ++++++++++++++++++------------------ > 2 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > index 7e459e839f8b..e2c175d608bb 100644 > --- a/include/linux/bpf_verifier.h > +++ b/include/linux/bpf_verifier.h > @@ -841,6 +841,7 @@ struct bpf_verifier_env { > char tmp_str_buf[TMP_STR_BUF_LEN]; > struct bpf_insn insn_buf[INSN_BUF_SIZE]; > struct bpf_insn epilogue_buf[INSN_BUF_SIZE]; > + struct bpf_scc_callchain callchain; Nit: maybe a comment here about this being a scratch buffer? > /* array of pointers to bpf_scc_info indexed by SCC id */ > struct bpf_scc_info **scc_info; > u32 scc_cnt; [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack 2025-07-02 19:03 ` Eduard Zingerman @ 2025-07-03 2:01 ` Yonghong Song 0 siblings, 0 replies; 9+ messages in thread From: Yonghong Song @ 2025-07-03 2:01 UTC (permalink / raw) To: Eduard Zingerman, bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team, Martin KaFai Lau, Arnd Bergmann, Jiri Olsa On 7/2/25 12:03 PM, Eduard Zingerman wrote: > On Wed, 2025-07-02 at 10:11 -0700, Yonghong Song wrote: >> Add a 'struct bpf_scc_callchain callchain' field in bpf_verifier_env. >> This way, the previous bpf_scc_callchain local variables can be >> replaced by taking address of env->callchain. This can reduce stack >> usage and fix the following error: >> kernel/bpf/verifier.c:19921:12: error: stack frame size (1368) exceeds limit (1280) in 'do_check' >> [-Werror,-Wframe-larger-than] >> >> Reported-by: Arnd Bergmann <arnd@kernel.org> >> Acked-by: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >> --- > Oh, well. > I liked stack allocation for callchain object, because it emphasized > its ephemeral by-value status. > > The changes lgtm, all places with callchain stack allocation replaced. > > Acked-by: Eduard Zingerman <eddyz87@gmail.com> > > [...] > >> include/linux/bpf_verifier.h | 1 + >> kernel/bpf/verifier.c | 36 ++++++++++++++++++------------------ >> 2 files changed, 19 insertions(+), 18 deletions(-) >> >> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >> index 7e459e839f8b..e2c175d608bb 100644 >> --- a/include/linux/bpf_verifier.h >> +++ b/include/linux/bpf_verifier.h >> @@ -841,6 +841,7 @@ struct bpf_verifier_env { >> char tmp_str_buf[TMP_STR_BUF_LEN]; >> struct bpf_insn insn_buf[INSN_BUF_SIZE]; >> struct bpf_insn epilogue_buf[INSN_BUF_SIZE]; >> + struct bpf_scc_callchain callchain; > Nit: maybe a comment here about this being a scratch buffer? Maybe I can just change the variable name from 'callchain' to 'callchain_buf' so it will be clear it is a scratch buffer? > >> /* array of pointers to bpf_scc_info indexed by SCC id */ >> struct bpf_scc_info **scc_info; >> u32 scc_cnt; > [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-03 2:03 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-02 17:11 [PATCH bpf-next v2 0/3] bpf: Reduce verifier stack frame size Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 1/3] bpf: Simplify assignment to struct bpf_insn pointer in do_misc_fixups() Yonghong Song 2025-07-02 18:58 ` Eduard Zingerman 2025-07-02 17:11 ` [PATCH bpf-next v2 2/3] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Yonghong Song 2025-07-02 20:33 ` Eduard Zingerman 2025-07-03 2:02 ` Yonghong Song 2025-07-02 17:11 ` [PATCH bpf-next v2 3/3] bpf: Avoid putting struct bpf_scc_callchain variables on the stack Yonghong Song 2025-07-02 19:03 ` Eduard Zingerman 2025-07-03 2:01 ` Yonghong Song
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.