All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* [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 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

* 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 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 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

* 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

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.