From: Jiri Olsa <olsajiri@gmail.com>
To: Yonghong Song <yonghong.song@linux.dev>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>,
Arnd Bergmann <arnd@kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Reduce stack frame size by using env->insn_buf for bpf insns
Date: Wed, 2 Jul 2025 10:54:15 +0200 [thread overview]
Message-ID: <aGTztxGRXTbD3lp9@krava> (raw)
In-Reply-To: <20250702053332.1991516-1-yonghong.song@linux.dev>
On Tue, Jul 01, 2025 at 10:33:32PM -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>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
lgtm
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> kernel/bpf/verifier.c | 194 ++++++++++++++++++++----------------------
> 1 file changed, 91 insertions(+), 103 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 90e688f81a48..29faef51065d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -20939,26 +20939,27 @@ static bool insn_is_cond_jump(u8 code)
> static void opt_hard_wire_dead_code_branches(struct bpf_verifier_env *env)
> {
> struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
> - struct bpf_insn ja = BPF_JMP_IMM(BPF_JA, 0, 0, 0);
> + struct bpf_insn *ja = env->insn_buf;
> struct bpf_insn *insn = env->prog->insnsi;
> const int insn_cnt = env->prog->len;
> int i;
>
> + *ja = BPF_JMP_IMM(BPF_JA, 0, 0, 0);
> for (i = 0; i < insn_cnt; i++, insn++) {
> if (!insn_is_cond_jump(insn->code))
> continue;
>
> if (!aux_data[i + 1].seen)
> - ja.off = insn->off;
> + ja->off = insn->off;
> else if (!aux_data[i + 1 + insn->off].seen)
> - ja.off = 0;
> + ja->off = 0;
> else
> continue;
>
> if (bpf_prog_is_offloaded(env->prog->aux))
> - bpf_prog_offload_replace_insn(env, i, &ja);
> + bpf_prog_offload_replace_insn(env, i, ja);
>
> - memcpy(insn, &ja, sizeof(ja));
> + memcpy(insn, ja, sizeof(*ja));
> }
> }
>
> @@ -21017,7 +21018,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;
> @@ -21195,13 +21198,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[0];
>
> - 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;
>
> @@ -21269,13 +21271,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[0];
>
> - 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;
>
> @@ -21945,13 +21946,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[0];
>
> - 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;
> @@ -21987,20 +21987,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[0];
> +
> + 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);
>
> - patchlet = isdiv ? chk_and_sdiv : chk_and_smod;
> - cnt = isdiv ? ARRAY_SIZE(chk_and_sdiv) : ARRAY_SIZE(chk_and_smod);
> + 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;
>
> @@ -22019,83 +22017,73 @@ 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[0];
> +
> + 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;
> + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1);
> + *patch++ = BPF_MOV32_REG(insn->dst_reg, insn->dst_reg);
> + cnt = (patch - insn_buf) - (is64 ? 2 : 0);
> + } 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;
> + *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1);
> + *patch++ = BPF_MOV32_REG(insn->dst_reg, insn->dst_reg);
> + cnt = (patch - insn_buf) - (is64 ? 2 : 0);
> }
>
> - 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
>
>
next prev parent reply other threads:[~2025-07-02 8:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-02 5:33 [PATCH bpf-next 1/2] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Yonghong Song
2025-07-02 5:33 ` [PATCH bpf-next 2/2] bpf: Avoid putting struct bpf_scc_callchain variables on the stack Yonghong Song
2025-07-02 8:49 ` Jiri Olsa
2025-07-02 8:54 ` Jiri Olsa [this message]
2025-07-02 9:28 ` [PATCH bpf-next 1/2] bpf: Reduce stack frame size by using env->insn_buf for bpf insns Arnd Bergmann
2025-07-02 15:09 ` Alexei Starovoitov
2025-07-02 15:29 ` Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aGTztxGRXTbD3lp9@krava \
--to=olsajiri@gmail.com \
--cc=andrii@kernel.org \
--cc=arnd@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox