diff mbox series

[v13,3/8] powerpc/bpf: fix buffer overflow in JIT for large BPF programs

Message ID e5fc39c4f3340791bec5e7a92e1d146d3ed6296e.1788154635.git.skb99@linux.ibm.com (mailing list archive)
State New
Headers show
Series powerpc/bpf: address missing verifier selftest coverage | expand

Commit Message

Saket Kumar Bhaskar Aug. 31, 2026, 7:16 a.m. UTC 
From: Abhishek Dubey <adubey@linux.ibm.com>

During size calculation in pass-0, exit_addr is 0 since addrs[fp->len]
is not yet populated. bpf_jit_emit_exit_insn() treats a zero exit_addr
as in-range and skips bpf_jit_build_epilogue(), so the alternate inline
epilogue instructions are not counted in alloclen.

In later passes, if the real exit_addr falls outside the 32MB branch
range, the full inline epilogue is emitted into the already-allocated
buffer, writing past its end and corrupting adjacent memory.

Fix by ensuring exit_addr is non-zero before treating it as in-range,
so pass-0 always falls through to bpf_jit_build_epilogue() and
conservatively accounts for all epilogue instructions in alloclen.
Also range check alt_exit_addr directly in the else-if condition.

Since exit_addr handling now falls through to the epilogue, two
related issues in bpf_int_jit_compile() must also be addressed:

1. Reset cgctx.alt_exit_addr before the second size-calculation pass.
   Without this, a stale alt_exit_addr from the first pass causes the
   second pass to emit a single jump instead of the full epilogue,
   undercounting alloclen and reintroducing the overflow.

2. Recompute addrs[fp->len] at the end of each code-generation pass.
   The larger pass-0 body can shrink in later passes as out-of-range
   exits settle into in-range jumps; a stale addrs[fp->len] would
   leave exit branches targeting past the real (shrunken) epilogue.

Because shrinkage in a later pass can move the epilogue offset, the
fixed two-pass loop is no longer sufficient: an exit that was out of
range in an earlier pass may fall in range once the epilogue offset
shrinks, shrinking the body further and overwriting the start of the
epilogue. Convert the code-generation loop to iterate until the
program size converges, bounded by CODEGEN_MAX_PASSES, and fail the
JIT if it does not converge.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/bpf/20260529015855.364704-2-adubey@linux.ibm.com/T/#mfcb23909d977b949727cca4f59ee56a13fd69b92
Fixes: d243b62b7bd3 ("powerpc64/bpf: Add support for bpf trampolines")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
Signed-off-by: Saket Kumar Bhaskar <skb99@linux.ibm.com>
Link: https://lore.kernel.org/bpf/20260529015855.364704-2-adubey@linux.ibm.com/T/#mfcb23909d977b949727cca4f59ee56a13fd69b92
Tested-by: Yeswanth Krishna Tellakula <yeswanth@linux.ibm.com>
---
 arch/powerpc/net/bpf_jit.h      |  7 +++++++
 arch/powerpc/net/bpf_jit_comp.c | 34 +++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 8 deletions(-)

Comments

bot+bpf-ci@kernel.org Aug. 31, 2026, 8:24 a.m. UTC | #1 
>     Fixes: d243b62b7bd3 ("powerpc64/bpf: Add support for bpf trampolines")

Does the Fixes: tag name the correct commit?

With Cc: stable@vger.kernel.org this fix will only reach v6.13+ and will
leave every stable tree from v5.18 to v6.12 carrying the same JIT buffer
overflow.

The faulty logic predates d243b62b7bd3. Looking at the parent of that commit:

git show d243b62b7bd3^:arch/powerpc/net/bpf_jit_comp.c

already contains the exact code being fixed:

    if (!exit_addr || is_offset_in_branch_range(exit_addr - (ctx->idx * 4))) {
            PPC_JMP(exit_addr);
    } else if (ctx->alt_exit_addr) {
            if (WARN_ON(!is_offset_in_branch_range((long)ctx->alt_exit_addr - (ctx->idx * 4))))
                    return -1;
            PPC_JMP(ctx->alt_exit_addr);
    } else {
            ctx->alt_exit_addr = ctx->idx * 4;
            bpf_jit_build_epilogue(image, ctx);
    }

The real introducing commit is 0ffdbce6f4a89bb7c0002904d6438ec83cf05ce7
("powerpc/bpf: Handle large branch ranges with BPF_EXIT"), merged in v5.18-rc1
(git describe --contains: v5.18-rc1~124^2~42). It created
bpf_jit_emit_exit_insn() with the '!exit_addr' early-out and replaced the
previous unconditional one-instruction emission:

    -               if (i != flen - 1)
    -                       PPC_JMP(exit_addr);
    +               if (i != flen - 1) {
    +                       ret = bpf_jit_emit_exit_insn(image, ctx, b2p[TMP_REG_1], exit_addr);

Before 0ffdbce6f4a8 a non-final BPF_EXIT always cost exactly one instruction in
every pass, so the sizing pass could not under-count and no overflow was
possible. After it, the sizing pass emits PPC_JMP(0) (1 instruction) while
later passes may emit a full inline epilogue, which is precisely the
under-count described in this commit message.

The commit d243b62b7bd3 only enlarged the epilogue (it added
bpf_jit_build_fentry_stubs()), making an already-existing overflow bigger.

Citing the ppc64-trampoline commit also wrongly implies PPC32 is unaffected.
The commit 0ffdbce6f4a8 made the identical change to
arch/powerpc/net/bpf_jit_comp32.c, so 32-bit powerpc has the same bug over the
same range.

A tag survey confirms the range: 'alt_exit_addr' has 0 occurrences in
arch/powerpc/net/ at v5.16 and v5.17, and 5 occurrences at v5.18, v5.19, v6.0,
v6.1 and v6.6.

Should the tag be:

  Fixes: 0ffdbce6f4a8 ("powerpc/bpf: Handle large branch ranges with BPF_EXIT")

> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 8f7501954d9f0..11981d2270a9d 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -99,11 +99,10 @@ void bpf_jit_build_fentry_stubs(u32 *image, struct codegen_context *ctx)
>
>  int bpf_jit_emit_exit_insn(u32 *image, struct codegen_context *ctx, int tmp_reg, long exit_addr)
>  {
> -	if (!exit_addr || is_offset_in_branch_range(exit_addr - (ctx->idx * 4))) {
> +	if (exit_addr && is_offset_in_branch_range(exit_addr - (long)(ctx->idx * 4))) {
>  		PPC_JMP(exit_addr);
> -	} else if (ctx->alt_exit_addr) {
> -		if (WARN_ON(!is_offset_in_branch_range((long)ctx->alt_exit_addr - (ctx->idx * 4))))
> -			return -1;
> +	} else if (ctx->alt_exit_addr && is_offset_in_branch_range(
> +			(long)(ctx->alt_exit_addr) - (long)(ctx->idx * 4))) {
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  		PPC_JMP(ctx->alt_exit_addr);

With alt_exit_addr defined as unsigned int, does the expression
(long)(ctx->alt_exit_addr) - (long)(ctx->idx * 4) produce a different result
than (long)ctx->alt_exit_addr - (ctx->idx * 4) due to how integer promotion
works?

In v7 review, Hari Bathini pointed out that the cast placement may affect
integer promotion semantics and requested restoring the earlier syntax with the
cast only on alt_exit_addr:

https://lore.kernel.org/bpf/038a115b-e2a5-4ecf-82b1-3689535e986b@linux.ibm.com

While the current code has passed testing, the concern about integer promotion
semantics was never explicitly addressed.

[ ... ]

> @@ -399,7 +417,7 @@ int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, u32 *fimage, int pass
>  	u32 *fixup;
>
>  	/* Populate extable entries only in the last pass */
> -	if (pass != 2)
> +	if (pass < CODEGEN_MIN_PASSES)
>  		return 0;

Does the comment still match the code?

With the old 'if (pass != 2)' and a fixed two-pass loop, pass 2 was the last
pass, so the comment was accurate. With the new loop and 'pass <
CODEGEN_MIN_PASSES', both pass 2 and pass 3 can write extable entries, so
entries are populated in every pass from 2 onwards, not only in the last one.

Also, CODEGEN_MIN_PASSES is being reused here with a different meaning. In
arch/powerpc/net/bpf_jit.h it documents the minimum number of code-generation
passes, but here it is used as 'the first pass in which extable entries are
valid'. The two are logically independent - bumping CODEGEN_MIN_PASSES to 3 to
force more settling iterations would silently stop extable entries being
written in pass 2.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33368376983
diff mbox series

Patch

diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 35015d7ecb76..6d58df361648 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -14,6 +14,13 @@ 
 #include <asm/ppc-opcode.h>
 #include <linux/build_bug.h>
 
+/*
+ * We need at least 2 passes for proper code generation, and may need
+ * additional passes if code size changes between passes.
+ */
+#define CODEGEN_MIN_PASSES    2
+#define CODEGEN_MAX_PASSES    3
+
 #ifdef CONFIG_PPC64_ELF_ABI_V1
 #define FUNCTION_DESCR_SIZE	24
 #else
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 8f7501954d9f..11981d2270a9 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -99,11 +99,10 @@  void bpf_jit_build_fentry_stubs(u32 *image, struct codegen_context *ctx)
 
 int bpf_jit_emit_exit_insn(u32 *image, struct codegen_context *ctx, int tmp_reg, long exit_addr)
 {
-	if (!exit_addr || is_offset_in_branch_range(exit_addr - (ctx->idx * 4))) {
+	if (exit_addr && is_offset_in_branch_range(exit_addr - (long)(ctx->idx * 4))) {
 		PPC_JMP(exit_addr);
-	} else if (ctx->alt_exit_addr) {
-		if (WARN_ON(!is_offset_in_branch_range((long)ctx->alt_exit_addr - (ctx->idx * 4))))
-			return -1;
+	} else if (ctx->alt_exit_addr && is_offset_in_branch_range(
+			(long)(ctx->alt_exit_addr) - (long)(ctx->idx * 4))) {
 		PPC_JMP(ctx->alt_exit_addr);
 	} else {
 		ctx->alt_exit_addr = ctx->idx * 4;
@@ -274,6 +273,7 @@  struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 	 */
 	if (cgctx.seen & SEEN_TAILCALL || !is_offset_in_branch_range((long)cgctx.idx * 4)) {
 		cgctx.idx = 0;
+		cgctx.alt_exit_addr = 0;
 		if (bpf_jit_build_body(fp, NULL, NULL, &cgctx, addrs, 0, false))
 			goto out_err;
 	}
@@ -306,10 +306,13 @@  struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 	code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
 	fcode_base = (u32 *)(fimage + FUNCTION_DESCR_SIZE);
 
-	/* Code generation passes 1-2 */
-	for (pass = 1; pass < 3; pass++) {
+	/* Code generation passes 1-2+, loop until program size converges. */
+	for (pass = 1; pass <= CODEGEN_MAX_PASSES; pass++) {
+		u32 prev_proglen = proglen;
+
 		/* Now build the prologue, body code & epilogue for real. */
 		cgctx.idx = 0;
+		cgctx.exentry_idx = 0;
 		cgctx.alt_exit_addr = 0;
 		bpf_jit_build_prologue(code_base, &cgctx);
 		if (bpf_jit_build_body(fp, code_base, fcode_base, &cgctx, addrs, pass,
@@ -318,11 +321,26 @@  struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 			bpf_jit_binary_pack_free(fhdr, hdr);
 			goto out_err;
 		}
+		addrs[fp->len] = cgctx.idx * 4;
 		bpf_jit_build_epilogue(code_base, &cgctx);
 
+		proglen = cgctx.idx * 4;
+
 		if (bpf_jit_enable > 1)
 			pr_info("Pass %d: shrink = %d, seen = 0x%x\n", pass,
-				proglen - (cgctx.idx * 4), cgctx.seen);
+				prev_proglen - proglen, cgctx.seen);
+
+		/* Check if program size has converged, but ensure minimum passes */
+		if (pass >= CODEGEN_MIN_PASSES && proglen == prev_proglen)
+			break;
+
+		if (pass == CODEGEN_MAX_PASSES && proglen != prev_proglen) {
+			pr_err("BPF JIT: Program did not converge after %d passes\n",
+								CODEGEN_MAX_PASSES);
+			bpf_arch_text_copy(&fhdr->size, &hdr->size, sizeof(hdr->size));
+			bpf_jit_binary_pack_free(fhdr, hdr);
+			goto out_err;
+		}
 	}
 
 	if (bpf_jit_enable > 1)
@@ -399,7 +417,7 @@  int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, u32 *fimage, int pass
 	u32 *fixup;
 
 	/* Populate extable entries only in the last pass */
-	if (pass != 2)
+	if (pass < CODEGEN_MIN_PASSES)
 		return 0;
 
 	if (!fp->aux->extable ||