* [PATCH bpf-next 0/2] bpf: Fix x86 JIT stopping before jumps converge
@ 2026-10-21 14:21 Mykyta Yatsenko
2026-10-21 14:21 ` [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves Mykyta Yatsenko
2026-10-21 14:21 ` [PATCH bpf-next 2/2] selftests/bpf: Test JIT jumps which change at a repeated image length Mykyta Yatsenko
0 siblings, 2 replies; 5+ messages in thread
From: Mykyta Yatsenko @ 2026-10-21 14:21 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko
The x86 JIT stops its sizing passes once two passes give the same image
length. A backward jump can grow while other jumps shrink, so the length
may repeat with jump sizes still changing. The final pass then emits code
of a different size than was sized, hits "bpf_jit: fatal error" and the
program fails to load with -ENOTSUPP.
Patch 1 stops the passes only when no insn moved.
Patch 2 adds a selftest which fails to load without the fix.
This was hit in CI by cgroup_tcp_skb with the unpublished branch
inversion series, whose layout change happened to trigger it.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
Mykyta Yatsenko (2):
bpf: Stop JIT passes only when no insn moves
selftests/bpf: Test JIT jumps which change at a repeated image length
arch/x86/net/bpf_jit_comp.c | 25 +++++++---
tools/testing/selftests/bpf/prog_tests/jit_insns.c | 57 ++++++++++++++++++++++
2 files changed, 75 insertions(+), 7 deletions(-)
---
base-commit: 59d77affc1dc56c992308038e68d7041abef1b8d
change-id: 20261021-jit_size_fix-da316b8d492c
Best regards,
--
Mykyta Yatsenko <yatsenko@meta.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves 2026-10-21 14:21 [PATCH bpf-next 0/2] bpf: Fix x86 JIT stopping before jumps converge Mykyta Yatsenko @ 2026-10-21 14:21 ` Mykyta Yatsenko 2026-10-21 22:15 ` Alexei Starovoitov 2026-10-21 14:21 ` [PATCH bpf-next 2/2] selftests/bpf: Test JIT jumps which change at a repeated image length Mykyta Yatsenko 1 sibling, 1 reply; 5+ messages in thread From: Mykyta Yatsenko @ 2026-10-21 14:21 UTC (permalink / raw) To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko From: Mykyta Yatsenko <yatsenko@meta.com> A backward jump is sized from its target's current-pass address and its own previous-pass address, so it can grow while other jumps shrink. Two passes may then give the same image length with jumps still changing, and the image pass fails with "bpf_jit: fatal error". Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com> --- arch/x86/net/bpf_jit_comp.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 6a2fe3d0039d..b483cde866d9 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -355,6 +355,9 @@ struct jit_context { */ int tail_call_direct_label; int tail_call_indirect_label; + + /* Set when a pass moves the end of any insn */ + bool addrs_changed; }; /* Maximum number of bytes emitted while JITing one eBPF insn */ @@ -2574,6 +2577,13 @@ static int emit_unwind_ret_addr(u8 **pprog, u8 *ip) return 0; } +static void set_insn_end(struct jit_context *ctx, int *insn_end, int end) +{ + if (*insn_end != end) + ctx->addrs_changed = true; + *insn_end = end; +} + static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image, int oldproglen, struct jit_context *ctx, bool jmp_padding) { @@ -2702,7 +2712,8 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int * if (rw_image) memcpy(rw_image + proglen, temp, ilen); proglen += ilen; - addrs[0] = proglen; + ctx->addrs_changed = false; + set_insn_end(ctx, addrs, proglen); prog = temp; for (i = 1; i <= insn_cnt; i++, insn++) { @@ -3930,7 +3941,7 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int * memcpy(rw_image + proglen, temp, ilen); } proglen += ilen; - addrs[i] = proglen; + set_insn_end(ctx, addrs + i, proglen); prog = temp; } @@ -4977,10 +4988,10 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr skip_init_addrs: /* - * JITed image shrinks with every pass and the loop iterates - * until the image stops shrinking. Very large BPF programs - * may converge on the last pass. In such case do one more - * pass to emit the final image. + * JITed image usually shrinks with every pass, but individual + * jumps can grow, so the loop iterates until no insn moves. + * Very large BPF programs may converge on the last pass. In + * such case do one more pass to emit the final image. */ for (pass = 0; pass < MAX_PASSES || image; pass++) { if (!padding && pass >= PADDING_PASSES) @@ -5010,7 +5021,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr } break; } - if (proglen == oldproglen) { + if (!ctx.addrs_changed) { /* * The number of entries in extable is the number of BPF_LDX * insns that access kernel memory via "pointer to BTF type". -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves 2026-10-21 14:21 ` [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves Mykyta Yatsenko @ 2026-10-21 22:15 ` Alexei Starovoitov 2026-10-21 22:52 ` Mykyta Yatsenko 0 siblings, 1 reply; 5+ messages in thread From: Alexei Starovoitov @ 2026-10-21 22:15 UTC (permalink / raw) To: Mykyta Yatsenko, bpf, andrii, daniel, kernel-team, eddyz87, memxor Cc: Mykyta Yatsenko On Wed, Oct 21, 2026 at 07:21 AM Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> wrote: > A backward jump is sized from its target's current-pass address and its > own previous-pass address, so it can grow while other jumps shrink. Two > passes may then give the same image length with jumps still changing, > and the image pass fails with "bpf_jit: fatal error". can we fix the sizing of the backward jump instead ? It's the third time it bites us. commit 1612a981b766 ("bpf, x64: fix JIT emission for dead code") special cased ja -1 and commit c8831bdbfbab ("bpf, x64: Fix a jit convergence issue") limited imm8 to 123 because of it. With this patch jumps still grow, and the one that grows in a padding pass still ends in "unexpected jmp_cond padding: -4 bytes". do_jit() knows how far the current pass moved insn i. With that taken into account a backward jump is sized by the insns it jumps over, the same way as a forward one. Insn sizes start at 64 and only go down, so the image shrinks with every pass like the comment in bpf_int_jit_compile() says, and proglen == oldproglen means that nothing moved. Then no need for addrs_changed, for the ja -1 special case and for is_imm8_jmp_offset(). Something like this (compile tested only): diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 6a2fe3d..c20d7d3 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -97,56 +97,6 @@ static bool is_imm8(int value) return value <= 127 && value >= -128; } -/* - * Let us limit the positive offset to be <= 123. - * This is to ensure eventual jit convergence For the following patterns: - * ... - * pass4, final_proglen=4391: - * ... - * 20e: 48 85 ff test rdi,rdi - * 211: 74 7d je 0x290 - * 213: 48 8b 77 00 mov rsi,QWORD PTR [rdi+0x0] - * ... - * 289: 48 85 ff test rdi,rdi - * 28c: 74 17 je 0x2a5 - * 28e: e9 7f ff ff ff jmp 0x212 - * 293: bf 03 00 00 00 mov edi,0x3 - * Note that insn at 0x211 is 2-byte cond jump insn for offset 0x7d (-125) - * and insn at 0x28e is 5-byte jmp insn with offset -129. - * - * pass5, final_proglen=4392: - * ... - * 20e: 48 85 ff test rdi,rdi - * 211: 0f 84 80 00 00 00 je 0x297 - * 217: 48 8b 77 00 mov rsi,QWORD PTR [rdi+0x0] - * ... - * 28d: 48 85 ff test rdi,rdi - * 290: 74 1a je 0x2ac - * 292: eb 84 jmp 0x218 - * 294: bf 03 00 00 00 mov edi,0x3 - * Note that insn at 0x211 is 6-byte cond jump insn now since its offset - * becomes 0x80 based on previous round (0x293 - 0x213 = 0x80). - * At the same time, insn at 0x292 is a 2-byte insn since its offset is - * -124. - * - * pass6 will repeat the same code as in pass4 and this will prevent - * eventual convergence. - * - * To fix this issue, we need to break je (2->6 bytes) <-> jmp (5->2 bytes) - * cycle in the above. In the above example je offset <= 0x7c should work. - * - * For other cases, je <-> je needs offset <= 0x7b to avoid no convergence - * issue. For jmp <-> je and jmp <-> jmp cases, jmp offset <= 0x7c should - * avoid no convergence issue. - * - * Overall, let us limit the positive offset for 8bit cond/uncond jmp insn - * to maximum 123 (0x7b). This way, the jit pass can eventually converge. - */ -static bool is_imm8_jmp_offset(int value) -{ - return value <= 123 && value >= -128; -} - static bool is_simm32(s64 value) { return value == (s64)(s32)value; @@ -2298,6 +2248,17 @@ static void emit_priv_frame_ptr(u8 **pprog, void __percpu *priv_frame_ptr) #define INSN_SZ_DIFF (((addrs[i] - addrs[i - 1]) - (prog - temp))) +/* + * addrs[] below i are from this pass, addrs[i] and above are from the previous + * one. 'moved' is how far this pass moved the start of insn i. With it a jump + * in either direction is sized by the insns it jumps over. All insns start at + * 64 bytes, so none of them ever grows and the image shrinks with every pass. + */ +static int jmp_distance(const int *addrs, int i, int off, int moved) +{ + return addrs[i + off] - addrs[i] + (off < 0 ? moved : 0); +} + #define __LOAD_TCC_PTR(off) \ emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, off) /* mov rax, qword ptr [rbp - rounded_stack_depth - 16] */ @@ -2588,7 +2549,7 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int * u64 arena_vm_start = 0, user_vm_start = 0; void __percpu *priv_stack_ptr; int i, excnt = 0; - int ilen, proglen = 0; + int ilen, moved, proglen = 0; u8 *ip, *prog = temp; u32 stack_depth; int callee_saved_size; @@ -2702,6 +2663,7 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int * if (rw_image) memcpy(rw_image + proglen, temp, ilen); proglen += ilen; + moved = addrs[0] - proglen; addrs[0] = proglen; prog = temp; @@ -3747,8 +3709,8 @@ emit_cond_jmp: /* Convert BPF opcode to x86 */ default: /* to silence GCC warning */ return -EFAULT; } - jmp_offset = addrs[i + insn->off] - addrs[i]; - if (is_imm8_jmp_offset(jmp_offset)) { + jmp_offset = jmp_distance(addrs, i, insn->off, moved); + if (is_imm8(jmp_offset)) { if (jmp_padding) { /* To keep the jmp_offset valid, the extra bytes are * padded before the jump insn, so we subtract the @@ -3788,23 +3750,10 @@ emit_cond_jmp: /* Convert BPF opcode to x86 */ break; case BPF_JMP | BPF_JA: case BPF_JMP32 | BPF_JA: - if (BPF_CLASS(insn->code) == BPF_JMP) { - if (insn->off == -1) - /* -1 jmp instructions will always jump - * backwards two bytes. Explicitly handling - * this case avoids wasting too many passes - * when there are long sequences of replaced - * dead code. - */ - jmp_offset = -2; - else - jmp_offset = addrs[i + insn->off] - addrs[i]; - } else { - if (insn->imm == -1) - jmp_offset = -2; - else - jmp_offset = addrs[i + insn->imm] - addrs[i]; - } + if (BPF_CLASS(insn->code) == BPF_JMP) + jmp_offset = jmp_distance(addrs, i, insn->off, moved); + else + jmp_offset = jmp_distance(addrs, i, insn->imm, moved); if (!jmp_offset) { /* @@ -3833,7 +3782,7 @@ emit_cond_jmp: /* Convert BPF opcode to x86 */ break; } emit_jmp: - if (is_imm8_jmp_offset(jmp_offset)) { + if (is_imm8(jmp_offset)) { if (jmp_padding) { /* To avoid breaking jmp_offset, the extra bytes * are padded before the actual jmp insn, so @@ -3869,7 +3818,7 @@ emit_jmp: emit_unwind_ret_addr(&prog, ip)) return -EINVAL; if (seen_exit) { - jmp_offset = ctx->cleanup_addr - addrs[i]; + jmp_offset = ctx->cleanup_addr - addrs[i] + moved; goto emit_jmp; } seen_exit = true; @@ -3930,6 +3879,7 @@ emit_jmp: memcpy(rw_image + proglen, temp, ilen); } proglen += ilen; + moved = addrs[i] - proglen; addrs[i] = proglen; prog = temp; } pw-bot: cr ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves 2026-10-21 22:15 ` Alexei Starovoitov @ 2026-10-21 22:52 ` Mykyta Yatsenko 0 siblings, 0 replies; 5+ messages in thread From: Mykyta Yatsenko @ 2026-10-21 22:52 UTC (permalink / raw) To: Alexei Starovoitov, bpf, andrii, daniel, kernel-team, eddyz87, memxor Cc: Mykyta Yatsenko On 10/21/26 11:15 PM, Alexei Starovoitov wrote: > On Wed, Oct 21, 2026 at 07:21 AM Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> wrote: >> A backward jump is sized from its target's current-pass address and its >> own previous-pass address, so it can grow while other jumps shrink. Two >> passes may then give the same image length with jumps still changing, >> and the image pass fails with "bpf_jit: fatal error". > > can we fix the sizing of the backward jump instead ? > > It's the third time it bites us. > commit 1612a981b766 ("bpf, x64: fix JIT emission for dead code") > special cased ja -1 and > commit c8831bdbfbab ("bpf, x64: Fix a jit convergence issue") > limited imm8 to 123 because of it. > With this patch jumps still grow, and the one that grows in a padding > pass still ends in "unexpected jmp_cond padding: -4 bytes". > > do_jit() knows how far the current pass moved insn i. With that taken > into account a backward jump is sized by the insns it jumps over, the > same way as a forward one. Insn sizes start at 64 and only go down, so > the image shrinks with every pass like the comment in > bpf_int_jit_compile() says, and proglen == oldproglen means that > nothing moved. > Then no need for addrs_changed, for the ja -1 special case and > for is_imm8_jmp_offset(). > > Something like this (compile tested only): I'll send v2. Thanks for the suggestion. > pw-bot: cr ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Test JIT jumps which change at a repeated image length 2026-10-21 14:21 [PATCH bpf-next 0/2] bpf: Fix x86 JIT stopping before jumps converge Mykyta Yatsenko 2026-10-21 14:21 ` [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves Mykyta Yatsenko @ 2026-10-21 14:21 ` Mykyta Yatsenko 1 sibling, 0 replies; 5+ messages in thread From: Mykyta Yatsenko @ 2026-10-21 14:21 UTC (permalink / raw) To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko From: Mykyta Yatsenko <yatsenko@meta.com> Cover the case where two JIT passes give the same image length while jumps still change size, which used to fail with -ENOTSUPP on x86-64. Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com> --- tools/testing/selftests/bpf/prog_tests/jit_insns.c | 57 ++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/jit_insns.c b/tools/testing/selftests/bpf/prog_tests/jit_insns.c index dabefdb8a353..1abfe6161bb9 100644 --- a/tools/testing/selftests/bpf/prog_tests/jit_insns.c +++ b/tools/testing/selftests/bpf/prog_tests/jit_insns.c @@ -2881,6 +2881,61 @@ static void test_convergence(void) } } +/* + * On x86-64 a backward jump grows here while others shrink, so two passes + * give the same image length with jumps still changing. The fillers are 7 + * and 2 bytes of native code, jumps are never taken. + */ +static void test_same_len_not_converged(void) +{ + struct bpf_insn insns[] = { + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, IN0), + BPF_LDX_MEM(BPF_DW, BPF_REG_4, BPF_REG_1, IN1), + BPF_JMP_IMM(BPF_JA, 0, 0, 1), + BPF_JMP_IMM(BPF_JA, 0, 0, 29), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 20), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, -3), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 22), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 2), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 23), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_JMP_IMM(BPF_JA, 0, 0, -22), + BPF_MOV64_IMM(BPF_REG_2, -1), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 3), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_JMP32_REG(BPF_JEQ, BPF_REG_0, BPF_REG_4, 2), + BPF_JMP_IMM(BPF_JA, 0, 0, -22), + BPF_MOV32_IMM(BPF_REG_2, 0), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 10), + BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, RES), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + int fd; + + fd = load_prog(insns, ARRAY_SIZE(insns), "same len not converged"); + if (fd < 0) + return; + prog_run(fd, 1, 2, 11, "same len not converged"); + close(fd); +} + /* * Calls into a subprogram with all five arguments, which clobbers a subset * of r6-r9 and uses the deepest stack slot the budget leaves, directly or @@ -3087,6 +3142,8 @@ void test_jit_insns(void) test_max_jmp(); if (test__start_subtest("convergence")) test_convergence(); + if (test__start_subtest("same_len_not_converged")) + test_same_len_not_converged(); if (test__start_subtest("calls")) test_calls(); if (test__start_subtest("helper_calls")) -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-21 22:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-21 14:21 [PATCH bpf-next 0/2] bpf: Fix x86 JIT stopping before jumps converge Mykyta Yatsenko 2026-10-21 14:21 ` [PATCH bpf-next 1/2] bpf: Stop JIT passes only when no insn moves Mykyta Yatsenko 2026-10-21 22:15 ` Alexei Starovoitov 2026-10-21 22:52 ` Mykyta Yatsenko 2026-10-21 14:21 ` [PATCH bpf-next 2/2] selftests/bpf: Test JIT jumps which change at a repeated image length Mykyta Yatsenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox