[PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT
@ 2026-09-15  2:13 Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX " Nicholas Dudar
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

Add MOVSX, MEMSX, 32-bit-offset unconditional jumps and unconditional
byte swaps to the i386 JIT. MOVSX currently produces ordinary moves;
the other instructions are rejected by the backend.

The load-emitter extraction is separate from MEMSX so the code movement
and signed-load handling can be reviewed independently. MOVSX comes first
because context-access conversion can lower signed loads into ordinary
loads followed by MOVSX.

The separately posted i386 SDIV/SMOD work is independent of this series:
https://lore.kernel.org/bpf/20260731002950.179573-1-main.kalliope@gmail.com/

Tested on QEMU i386 with CONFIG_BPF_JIT_ALWAYS_ON=y. The full test_bpf
suite improves from 845/216 to 862/199 pass/fail: the 17 newly passing
cases cover these instructions, with no other result changes. Additional
verifier-loaded programs check full-width results and ordinary controls.

Of the remaining 199 failures, 8 are ALU32 SDIV/SMOD result mismatches.
The other 191 fail during program creation or runtime selection: 149
atomic cases, 37 ALU64 division/modulo cases (including a mixed ALU test),
4 long conditional jumps and 1 jump/gap stress case.

All five intermediate JIT objects build. A standalone emission harness
checks ordinary-load byte equivalence across the extraction and compares
MOVSX staging with the earlier emitter, accounting for the removed
self-moves. No physical hardware was tested.

Nicholas Dudar (5):
  bpf, x86: Add support for BPF_MOVSX in the i386 JIT
  bpf, x86: Extract the i386 JIT load emitter
  bpf, x86: Add BPF_MEMSX support to the i386 JIT
  bpf, x86: Add BPF_JMP32 | BPF_JA support to the i386 JIT
  bpf, x86: Add unconditional byte swap to the i386 JIT

 arch/x86/net/bpf_jit_comp32.c | 227 ++++++++++++++++++++++++----------
 1 file changed, 161 insertions(+), 66 deletions(-)

base-commit: 29fcf9ce4bbdf4cc339def776860e1c0095bca98

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX in the i386 JIT
  2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
@ 2026-09-15  2:13 ` Nicholas Dudar
  2026-09-15  3:12   ` bot+bpf-ci
  2026-09-15  2:13 ` [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter Nicholas Dudar
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

The i386 JIT lowers register BPF_MOVSX as an ordinary move because
do_jit() does not inspect insn->off. The generated code therefore copies
the source instead of sign-extending the selected low bits. Context-access
conversion can also introduce MOVSX while lowering signed loads, so
rejecting BPF_MEMSX does not protect those paths.

Stage the source low word in EAX and use the native byte and word
sign-extension instructions. Derive the high word from the sign bit for
ALU64 and retain verifier-managed zero extension for ALU32.

Assisted-by: Codex:gpt-6
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 51 +++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 852baf2e4db4..9cbeabac4f27 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -266,6 +266,51 @@ static inline void emit_ia32_mov_r64(const bool is64, const u8 dst[],
 		emit_ia32_mov_i(dst_hi, 0, dstk, pprog);
 }
 
+/* dst = sign_extend(src, insn->off) */
+static inline void emit_ia32_movsx_r64(const struct bpf_insn *insn, u8 **pprog,
+				       const struct bpf_prog_aux *aux)
+{
+	const u8 *dst = bpf2ia32[insn->dst_reg];
+	const u8 *src = bpf2ia32[insn->src_reg];
+	bool dstk = insn->dst_reg != BPF_REG_AX;
+	bool sstk = insn->src_reg != BPF_REG_AX;
+	bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
+	u8 *prog = *pprog;
+	int cnt = 0;
+
+	if (sstk)
+		/* mov eax,dword ptr [ebp+off] */
+		EMIT3(0x8B, add_2reg(0x40, IA32_EBP, IA32_EAX),
+		      STACK_VAR(src_lo));
+	else
+		/* mov eax,src_lo */
+		EMIT2(0x89, add_2reg(0xC0, IA32_EAX, src_lo));
+
+	switch (insn->off) {
+	case 8:
+		/* movsx eax,al */
+		EMIT3(0x0F, 0xBE, 0xC0);
+		break;
+	case 16:
+		/* movsx eax,ax */
+		EMIT3(0x0F, 0xBF, 0xC0);
+		break;
+	case 32:
+		/* EAX already holds the low word; CDQ supplies the high word. */
+		break;
+	}
+
+	emit_ia32_mov_r(dst_lo, IA32_EAX, dstk, false, &prog);
+	if (is64) {
+		EMIT1(0x99); /* cdq */
+		emit_ia32_mov_r(dst_hi, IA32_EDX, dstk, false, &prog);
+	} else if (!aux->verifier_zext) {
+		emit_ia32_mov_i(dst_hi, 0, dstk, &prog);
+	}
+
+	*pprog = prog;
+}
+
 /* Sign extended move */
 static inline void emit_ia32_mov_i64(const bool is64, const u8 dst[],
 				     const u32 val, bool dstk, u8 **pprog)
@@ -1697,6 +1742,12 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 					emit_ia32_mov_i(dst_hi, 0, dstk, &prog);
 					break;
 				}
+				if (insn->off == 8 || insn->off == 16 ||
+				    (is64 && insn->off == 32)) {
+					emit_ia32_movsx_r64(insn, &prog,
+							    bpf_prog->aux);
+					break;
+				}
 				emit_ia32_mov_r64(is64, dst, src, dstk, sstk,
 						  &prog, bpf_prog->aux);
 				break;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX in the i386 JIT
  2026-09-15  2:13 ` [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX " Nicholas Dudar
@ 2026-09-15  3:12   ` bot+bpf-ci
  0 siblings, 0 replies; 10+ messages in thread
From: bot+bpf-ci @ 2026-09-15  3:12 UTC (permalink / raw)
  To: main.kalliope, ast, daniel, andrii, eddyz87, memxor, udknight,
	tglx, mingo, bp, dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3336 bytes --]

>     bpf, x86: Add support for BPF_MOVSX in the i386 JIT
>
>     The i386 JIT lowers register BPF_MOVSX as an ordinary move because
>     do_jit() does not inspect insn->off. The generated code therefore copies
>     the source instead of sign-extending the selected low bits. Context-access
>     conversion can also introduce MOVSX while lowering signed loads, so
>     rejecting BPF_MEMSX does not protect those paths.
>
>     Stage the source low word in EAX and use the native byte and word
>     sign-extension instructions. Derive the high word from the sign bit for
>     ALU64 and retain verifier-managed zero extension for ALU32.
>
>     Assisted-by: Codex:gpt-6
>     Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>

This looks like a bug fix for MOVSX support that was added to x86_64 but
not i386. Should this include:

  Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")

> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 852baf2e4db4d..9cbeabac4f27f 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -266,6 +266,51 @@ static inline void emit_ia32_mov_r64(const bool is64, const u8 dst[],
>  		emit_ia32_mov_i(dst_hi, 0, dstk, pprog);
>  }
>
> +/* dst = sign_extend(src, insn->off) */
> +static inline void emit_ia32_movsx_r64(const struct bpf_insn *insn, u8 **pprog,
> +				       const struct bpf_prog_aux *aux)
> +{
> +	const u8 *dst = bpf2ia32[insn->dst_reg];
> +	const u8 *src = bpf2ia32[insn->src_reg];
> +	bool dstk = insn->dst_reg != BPF_REG_AX;
> +	bool sstk = insn->src_reg != BPF_REG_AX;
> +	bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;

This isn't a bug, but would it read more consistently with the
neighbouring emitters to pass is64/dst/src/dstk/sstk as parameters?

Looking at the other ALU emitters in this file, they take the
already-decoded operands directly:

  emit_ia32_mov_r64(is64, dst, src, dstk, sstk, pprog, aux)
  emit_ia32_alu_r64(...)
  emit_ia32_mul_r64(...)

The new helper re-derives these five values from insn, even though
do_jit already has them in scope at the call site (lines 1719-1724).
The insn->off validation also happens twice: once in the do_jit gate
and again in the helper's switch.

> +	u8 *prog = *pprog;
> +	int cnt = 0;
> +
> +	if (sstk)
> +		/* mov eax,dword ptr [ebp+off] */
> +		EMIT3(0x8B, add_2reg(0x40, IA32_EBP, IA32_EAX),
> +		      STACK_VAR(src_lo));
> +	else
> +		/* mov eax,src_lo */
> +		EMIT2(0x89, add_2reg(0xC0, IA32_EAX, src_lo));
> +
> +	switch (insn->off) {
> +	case 8:
> +		/* movsx eax,al */
> +		EMIT3(0x0F, 0xBE, 0xC0);
> +		break;
> +	case 16:
> +		/* movsx eax,ax */
> +		EMIT3(0x0F, 0xBF, 0xC0);
> +		break;
> +	case 32:
> +		/* EAX already holds the low word; CDQ supplies the high word. */
> +		break;
> +	}
> +
> +	emit_ia32_mov_r(dst_lo, IA32_EAX, dstk, false, &prog);
> +	if (is64) {
> +		EMIT1(0x99); /* cdq */
> +		emit_ia32_mov_r(dst_hi, IA32_EDX, dstk, false, &prog);
> +	} else if (!aux->verifier_zext) {
> +		emit_ia32_mov_i(dst_hi, 0, dstk, &prog);
> +	}
> +
> +	*pprog = prog;
> +}

[ ... ]


---
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/34921540254

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter
  2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX " Nicholas Dudar
@ 2026-09-15  2:13 ` Nicholas Dudar
  2026-09-15  3:12   ` bot+bpf-ci
  2026-09-15  2:13 ` [PATCH bpf-next 3/5] bpf, x86: Add BPF_MEMSX support to the i386 JIT Nicholas Dudar
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

Move ordinary LDX lowering into an instruction-owned helper. Keep address
staging, displacement encoding and register-pair completion together,
including verifier-managed zero extension for narrow loads.

Use the helper for the existing BPF_MEM cases before adding signed loads.
This separates the code movement from the MEMSX instruction handling.
The emitted instructions are preserved.

Assisted-by: Codex:gpt-6
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 148 +++++++++++++++++++---------------
 1 file changed, 84 insertions(+), 64 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 9cbeabac4f27..6839c233cb73 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -1214,6 +1214,89 @@ static inline void emit_ia32_mul_i64(const u8 dst[], const u32 val,
 	*pprog = prog;
 }
 
+static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
+			  const struct bpf_prog_aux *aux)
+{
+	const u8 *dst = bpf2ia32[insn->dst_reg];
+	const u8 *src = bpf2ia32[insn->src_reg];
+	bool dstk = insn->dst_reg != BPF_REG_AX;
+	bool sstk = insn->src_reg != BPF_REG_AX;
+	u8 *prog = *pprog;
+	int cnt = 0;
+
+	/* Stage the address in EAX; dst may alias src. */
+	if (sstk)
+		/* mov eax,dword ptr [ebp+off] */
+		EMIT3(0x8B, add_2reg(0x40, IA32_EBP, IA32_EAX),
+		      STACK_VAR(src_lo));
+	else
+		/* mov eax,src_lo */
+		EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_EAX));
+
+	switch (BPF_SIZE(insn->code)) {
+	case BPF_B:
+		/* movzx edx,byte ptr [eax+off] */
+		EMIT2(0x0F, 0xB6);
+		break;
+	case BPF_H:
+		/* movzx edx,word ptr [eax+off] */
+		EMIT2(0x0F, 0xB7);
+		break;
+	case BPF_W:
+	case BPF_DW:
+		/* mov edx,dword ptr [eax+off] */
+		EMIT1(0x8B);
+		break;
+	}
+
+	if (is_imm8(insn->off))
+		EMIT2(add_2reg(0x40, IA32_EAX, IA32_EDX), insn->off);
+	else
+		EMIT1_off32(add_2reg(0x80, IA32_EAX, IA32_EDX), insn->off);
+
+	if (dstk)
+		/* mov dword ptr [ebp+off],edx */
+		EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
+		      STACK_VAR(dst_lo));
+	else
+		/* mov dst_lo,edx */
+		EMIT2(0x89, add_2reg(0xC0, dst_lo, IA32_EDX));
+
+	switch (BPF_SIZE(insn->code)) {
+	case BPF_B:
+	case BPF_H:
+	case BPF_W:
+		if (aux->verifier_zext)
+			break;
+		if (dstk) {
+			/* mov dword ptr [ebp+off],0 */
+			EMIT3(0xC7, add_1reg(0x40, IA32_EBP),
+			      STACK_VAR(dst_hi));
+			EMIT(0x0, 4);
+		} else {
+			/* xor dst_hi,dst_hi */
+			EMIT2(0x33, add_2reg(0xC0, dst_hi, dst_hi));
+		}
+		break;
+	case BPF_DW:
+		/* mov edx,dword ptr [eax+off+4] */
+		EMIT2_off32(0x8B, add_2reg(0x80, IA32_EAX, IA32_EDX),
+			    insn->off + 4);
+		if (dstk)
+			/* mov dword ptr [ebp+off],edx */
+			EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
+			      STACK_VAR(dst_hi));
+		else
+			/* mov dst_hi,edx */
+			EMIT2(0x89, add_2reg(0xC0, dst_hi, IA32_EDX));
+		break;
+	default:
+		break;
+	}
+
+	*pprog = prog;
+}
+
 static int bpf_size_to_x86_bytes(int bpf_size)
 {
 	if (bpf_size == BPF_W)
@@ -2065,70 +2148,7 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 		case BPF_LDX | BPF_MEM | BPF_H:
 		case BPF_LDX | BPF_MEM | BPF_W:
 		case BPF_LDX | BPF_MEM | BPF_DW:
-			if (sstk)
-				/* mov eax,dword ptr [ebp+off] */
-				EMIT3(0x8B, add_2reg(0x40, IA32_EBP, IA32_EAX),
-				      STACK_VAR(src_lo));
-			else
-				/* mov eax,dword ptr [ebp+off] */
-				EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_EAX));
-
-			switch (BPF_SIZE(code)) {
-			case BPF_B:
-				EMIT2(0x0F, 0xB6); break;
-			case BPF_H:
-				EMIT2(0x0F, 0xB7); break;
-			case BPF_W:
-			case BPF_DW:
-				EMIT(0x8B, 1); break;
-			}
-
-			if (is_imm8(insn->off))
-				EMIT2(add_2reg(0x40, IA32_EAX, IA32_EDX),
-				      insn->off);
-			else
-				EMIT1_off32(add_2reg(0x80, IA32_EAX, IA32_EDX),
-					    insn->off);
-
-			if (dstk)
-				/* mov dword ptr [ebp+off],edx */
-				EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
-				      STACK_VAR(dst_lo));
-			else
-				/* mov dst_lo,edx */
-				EMIT2(0x89, add_2reg(0xC0, dst_lo, IA32_EDX));
-			switch (BPF_SIZE(code)) {
-			case BPF_B:
-			case BPF_H:
-			case BPF_W:
-				if (bpf_prog->aux->verifier_zext)
-					break;
-				if (dstk) {
-					EMIT3(0xC7, add_1reg(0x40, IA32_EBP),
-					      STACK_VAR(dst_hi));
-					EMIT(0x0, 4);
-				} else {
-					/* xor dst_hi,dst_hi */
-					EMIT2(0x33,
-					      add_2reg(0xC0, dst_hi, dst_hi));
-				}
-				break;
-			case BPF_DW:
-				EMIT2_off32(0x8B,
-					    add_2reg(0x80, IA32_EAX, IA32_EDX),
-					    insn->off + 4);
-				if (dstk)
-					EMIT3(0x89,
-					      add_2reg(0x40, IA32_EBP,
-						       IA32_EDX),
-					      STACK_VAR(dst_hi));
-				else
-					EMIT2(0x89,
-					      add_2reg(0xC0, dst_hi, IA32_EDX));
-				break;
-			default:
-				break;
-			}
+			emit_ia32_ldx(insn, &prog, bpf_prog->aux);
 			break;
 		/* call */
 		case BPF_JMP | BPF_CALL:

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter
  2026-09-15  2:13 ` [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter Nicholas Dudar
@ 2026-09-15  3:12   ` bot+bpf-ci
  0 siblings, 0 replies; 10+ messages in thread
From: bot+bpf-ci @ 2026-09-15  3:12 UTC (permalink / raw)
  To: main.kalliope, ast, daniel, andrii, eddyz87, memxor, udknight,
	tglx, mingo, bp, dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2931 bytes --]

> bpf, x86: Extract the i386 JIT load emitter
>
> Move ordinary LDX lowering into an instruction-owned helper. Keep address
> staging, displacement encoding and register-pair completion together,
> including verifier-managed zero extension for narrow loads.
>
> Use the helper for the existing BPF_MEM cases before adding signed loads.
> This separates the code movement from the MEMSX instruction handling.
> The emitted instructions are preserved.

This isn't a bug, but could the middle paragraph be trimmed?  It
enumerates what the moved block contains rather than adding rationale
beyond what the diff already shows.  Would it read more clearly if the
changelog led with why the helper is being split out ahead of the MEMSX
patch, and used "No functional change intended" instead of "The emitted
instructions are preserved"?

> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 9cbeabac4f27f..6839c233cb734 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c

[ ... ]

> @@ -1214,6 +1214,89 @@ static inline void emit_ia32_mul_i64(const u8 dst[], const u32 val,
>  	*pprog = prog;
>  }
>
> +static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
> +			  const struct bpf_prog_aux *aux)
> +{
> +	const u8 *dst = bpf2ia32[insn->dst_reg];
> +	const u8 *src = bpf2ia32[insn->src_reg];
> +	bool dstk = insn->dst_reg != BPF_REG_AX;
> +	bool sstk = insn->src_reg != BPF_REG_AX;
> +	u8 *prog = *pprog;
> +	int cnt = 0;
> +
> +	/* Stage the address in EAX; dst may alias src. */
> +	if (sstk)
> +		/* mov eax,dword ptr [ebp+off] */
> +		EMIT3(0x8B, add_2reg(0x40, IA32_EBP, IA32_EAX),
> +		      STACK_VAR(src_lo));
> +	else
> +		/* mov eax,src_lo */
> +		EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_EAX));
> +
> +	switch (BPF_SIZE(insn->code)) {
> +	case BPF_B:
> +		/* movzx edx,byte ptr [eax+off] */
> +		EMIT2(0x0F, 0xB6);
> +		break;
> +	case BPF_H:
> +		/* movzx edx,word ptr [eax+off] */
> +		EMIT2(0x0F, 0xB7);
> +		break;
> +	case BPF_W:
> +	case BPF_DW:
> +		/* mov edx,dword ptr [eax+off] */
> +		EMIT1(0x8B);
> +		break;
> +	}

This isn't a bug, but since the patch is meant to be a pure move, would
it read more easily as a verbatim move plus a separate follow-up for the
cosmetic changes?  The moved block is reworked here: the three case arms
are reflowed from the original one-line form, EMIT(0x8B, 1) becomes
EMIT1(0x8B), several EMITs gain new mnemonic comments, and the
pre-existing comment is corrected from "mov eax,dword ptr [ebp+off]" to
"mov eax,src_lo".  Each edit is an improvement, but folding them into
the movement makes "emission is unchanged" harder to confirm by reading
the diff.

[ ... ]


---
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/34921540254

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 3/5] bpf, x86: Add BPF_MEMSX support to the i386 JIT
  2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX " Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter Nicholas Dudar
@ 2026-09-15  2:13 ` Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA " Nicholas Dudar
  2026-09-15  2:13 ` [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap " Nicholas Dudar
  4 siblings, 0 replies; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

The i386 JIT rejects BPF_MEMSX byte, halfword and word loads.

Use signed byte and halfword loads, then extend the loaded low word's
sign into the upper word for all three MEMSX widths. MEMSX defines a
64-bit result, so complete both halves regardless of verifier_zext.
Ordinary loads retain their existing zero-extension handling.

Assisted-by: Codex:gpt-6
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 71 ++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 27 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 6839c233cb73..bbe48b15850f 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -1221,6 +1221,7 @@ static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
 	const u8 *src = bpf2ia32[insn->src_reg];
 	bool dstk = insn->dst_reg != BPF_REG_AX;
 	bool sstk = insn->src_reg != BPF_REG_AX;
+	bool sign = BPF_MODE(insn->code) == BPF_MEMSX;
 	u8 *prog = *pprog;
 	int cnt = 0;
 
@@ -1235,12 +1236,12 @@ static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
 
 	switch (BPF_SIZE(insn->code)) {
 	case BPF_B:
-		/* movzx edx,byte ptr [eax+off] */
-		EMIT2(0x0F, 0xB6);
+		/* movsx/movzx edx,byte ptr [eax+off] */
+		EMIT2(0x0F, sign ? 0xBE : 0xB6);
 		break;
 	case BPF_H:
-		/* movzx edx,word ptr [eax+off] */
-		EMIT2(0x0F, 0xB7);
+		/* movsx/movzx edx,word ptr [eax+off] */
+		EMIT2(0x0F, sign ? 0xBF : 0xB7);
 		break;
 	case BPF_W:
 	case BPF_DW:
@@ -1262,26 +1263,10 @@ static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
 		/* mov dst_lo,edx */
 		EMIT2(0x89, add_2reg(0xC0, dst_lo, IA32_EDX));
 
-	switch (BPF_SIZE(insn->code)) {
-	case BPF_B:
-	case BPF_H:
-	case BPF_W:
-		if (aux->verifier_zext)
-			break;
-		if (dstk) {
-			/* mov dword ptr [ebp+off],0 */
-			EMIT3(0xC7, add_1reg(0x40, IA32_EBP),
-			      STACK_VAR(dst_hi));
-			EMIT(0x0, 4);
-		} else {
-			/* xor dst_hi,dst_hi */
-			EMIT2(0x33, add_2reg(0xC0, dst_hi, dst_hi));
-		}
-		break;
-	case BPF_DW:
-		/* mov edx,dword ptr [eax+off+4] */
-		EMIT2_off32(0x8B, add_2reg(0x80, IA32_EAX, IA32_EDX),
-			    insn->off + 4);
+	/* MEMSX defines both halves, regardless of verifier_zext. */
+	if (sign) {
+		/* sar edx,31 */
+		EMIT3(0xC1, add_1reg(0xF8, IA32_EDX), 31);
 		if (dstk)
 			/* mov dword ptr [ebp+off],edx */
 			EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
@@ -1289,9 +1274,38 @@ static void emit_ia32_ldx(const struct bpf_insn *insn, u8 **pprog,
 		else
 			/* mov dst_hi,edx */
 			EMIT2(0x89, add_2reg(0xC0, dst_hi, IA32_EDX));
-		break;
-	default:
-		break;
+	} else {
+		switch (BPF_SIZE(insn->code)) {
+		case BPF_B:
+		case BPF_H:
+		case BPF_W:
+			if (aux->verifier_zext)
+				break;
+			if (dstk) {
+				/* mov dword ptr [ebp+off],0 */
+				EMIT3(0xC7, add_1reg(0x40, IA32_EBP),
+				      STACK_VAR(dst_hi));
+				EMIT(0x0, 4);
+			} else {
+				/* xor dst_hi,dst_hi */
+				EMIT2(0x33, add_2reg(0xC0, dst_hi, dst_hi));
+			}
+			break;
+		case BPF_DW:
+			/* mov edx,dword ptr [eax+off+4] */
+			EMIT2_off32(0x8B, add_2reg(0x80, IA32_EAX, IA32_EDX),
+				    insn->off + 4);
+			if (dstk)
+				/* mov dword ptr [ebp+off],edx */
+				EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
+				      STACK_VAR(dst_hi));
+			else
+				/* mov dst_hi,edx */
+				EMIT2(0x89, add_2reg(0xC0, dst_hi, IA32_EDX));
+			break;
+		default:
+			break;
+		}
 	}
 
 	*pprog = prog;
@@ -2148,6 +2162,9 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 		case BPF_LDX | BPF_MEM | BPF_H:
 		case BPF_LDX | BPF_MEM | BPF_W:
 		case BPF_LDX | BPF_MEM | BPF_DW:
+		case BPF_LDX | BPF_MEMSX | BPF_B:
+		case BPF_LDX | BPF_MEMSX | BPF_H:
+		case BPF_LDX | BPF_MEMSX | BPF_W:
 			emit_ia32_ldx(insn, &prog, bpf_prog->aux);
 			break;
 		/* call */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA support to the i386 JIT
  2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
                   ` (2 preceding siblings ...)
  2026-09-15  2:13 ` [PATCH bpf-next 3/5] bpf, x86: Add BPF_MEMSX support to the i386 JIT Nicholas Dudar
@ 2026-09-15  2:13 ` Nicholas Dudar
  2026-09-15  3:12   ` bot+bpf-ci
  2026-09-15  2:13 ` [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap " Nicholas Dudar
  4 siblings, 1 reply; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

The i386 JIT rejects BPF_JMP32 | BPF_JA, which encodes its signed jump
displacement in imm rather than the 16-bit off field.

Select the displacement by instruction class and reuse the existing
unconditional jump lowering, including its self-jump and no-op handling.
The shared unconditional-jump path handles instruction zero separately:
its start is the prologue boundary, which has no preceding entry in
the instruction-end address map.

Assisted-by: Codex:gpt-6
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index bbe48b15850f..2d8a3a961e91 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -1821,6 +1821,7 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 		const u8 *src = bpf2ia32[insn->src_reg];
 		const u8 *r0 = bpf2ia32[BPF_REG_0];
 		s64 jmp_offset;
+		s32 ja_off;
 		u8 jmp_cond;
 		int ilen;
 		u8 *func;
@@ -2522,7 +2523,9 @@ emit_cond_jmp_signed:	/* Check the condition for low 32-bit comparison */
 			break;
 		}
 		case BPF_JMP | BPF_JA:
-			if (insn->off == -1)
+		case BPF_JMP32 | BPF_JA:
+			ja_off = BPF_CLASS(code) == BPF_JMP32 ? imm32 : insn->off;
+			if (ja_off == -1)
 				/* -1 jmp instructions will always jump
 				 * backwards two bytes. Explicitly handling
 				 * this case avoids wasting too many passes
@@ -2530,8 +2533,10 @@ emit_cond_jmp_signed:	/* Check the condition for low 32-bit comparison */
 				 * dead code.
 				 */
 				jmp_offset = -2;
+			else if (i + ja_off == -1)
+				jmp_offset = PROLOGUE_SIZE - addrs[i];
 			else
-				jmp_offset = addrs[i + insn->off] - addrs[i];
+				jmp_offset = addrs[i + ja_off] - addrs[i];
 
 			if (!jmp_offset)
 				/* Optimize out nop jumps */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA support to the i386 JIT
  2026-09-15  2:13 ` [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA " Nicholas Dudar
@ 2026-09-15  3:12   ` bot+bpf-ci
  0 siblings, 0 replies; 10+ messages in thread
From: bot+bpf-ci @ 2026-09-15  3:12 UTC (permalink / raw)
  To: main.kalliope, ast, daniel, andrii, eddyz87, memxor, udknight,
	tglx, mingo, bp, dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

> Select the displacement by instruction class and reuse the existing
> unconditional jump lowering, including its self-jump and no-op handling.
> The shared unconditional-jump path handles instruction zero separately:
> its start is the prologue boundary, which has no preceding entry in
> the instruction-end address map.

The new arm at arch/x86/net/bpf_jit_comp32.c line 2536:

    else if (i + ja_off == -1)
        jmp_offset = PROLOGUE_SIZE - addrs[i];

This isn't just new-opcode handling. Before this patch, BPF_JMP | BPF_JA
with i + insn->off == -1 would execute:

    jmp_offset = addrs[i + insn->off] - addrs[i];

which reads addrs[-1], one int before the kmalloc_array(*addrs, prog->len)
allocation at line 2627. This happens because the i386 JIT uses a 0-based
addrs[] array where addrs[i] is the end offset of instruction i (line 2603),
with no entry for the start of instruction 0. The x86-64 JIT avoids this by
using a 1-based array with addrs[0] = proglen after the prologue
(bpf_jit_comp.c:2088).

The out-of-bounds read is reachable when a privileged program has an
unconditional back-edge to instruction 0, for example:

    0: r0 = *(u32 *)(r1 + 0);
    1: if r0 == 0 goto +1;
    2: goto -3;
    3: exit

At instruction 2, i=2, off=-3, target is instruction 0, so i+off == -1.
push_insn() (kernel/bpf/cfg.c:113) only rejects targets outside
[0, prog->len-1], so -1 is exactly the boundary the old code got wrong.

Should the changelog say explicitly that the instruction-zero arm fixes
an existing out-of-bounds addrs[-1] read for BPF_JMP | BPF_JA? Would it
make sense to split this into a separate patch with a Fixes: tag so it can
be backported independently of the new gotol support?

Also, the identical boundary bug remains in the conditional-jump paths that
were left untouched. The pattern addrs[i + insn->off] appears at lines 2454,
2503, and 2516, so if rX == Y goto <insn 0> still reads addrs[-1]. Does it
make sense to fix all the jump paths together to keep the file consistent?

---
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/34921540254

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap to the i386 JIT
  2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
                   ` (3 preceding siblings ...)
  2026-09-15  2:13 ` [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA " Nicholas Dudar
@ 2026-09-15  2:13 ` Nicholas Dudar
  2026-09-15  3:12   ` bot+bpf-ci
  4 siblings, 1 reply; 10+ messages in thread
From: Nicholas Dudar @ 2026-09-15  2:13 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, udknight, tglx, mingo, bp,
	dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw

The i386 JIT rejects unconditional BPF byte-swap instructions, which use
BPF_ALU64 | BPF_END.

Route them through the existing big-endian conversion emitter. On
little-endian i386, it provides the same 16-, 32- and 64-bit byte swaps.
The verifier treats sub-64-bit BPF_END results as 32-bit definitions and
inserts zero extension where the upper word is used. The emitter clears
that word itself when verifier_zext is disabled.

Assisted-by: Codex:gpt-6
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 2d8a3a961e91..bdba05b72ef3 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -2037,6 +2037,8 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 			break;
 		/* dst = htobe(dst) */
 		case BPF_ALU | BPF_END | BPF_FROM_BE:
+		/* dst = bswap(dst) */
+		case BPF_ALU64 | BPF_END | BPF_FROM_LE:
 			emit_ia32_to_be_r64(dst, imm32, dstk, &prog,
 					    bpf_prog->aux);
 			break;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap to the i386 JIT
  2026-09-15  2:13 ` [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap " Nicholas Dudar
@ 2026-09-15  3:12   ` bot+bpf-ci
  0 siblings, 0 replies; 10+ messages in thread
From: bot+bpf-ci @ 2026-09-15  3:12 UTC (permalink / raw)
  To: main.kalliope, ast, daniel, andrii, eddyz87, memxor, udknight,
	tglx, mingo, bp, dave.hansen, x86
  Cc: martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai, hpa,
	bpf, linux-kernel, visitorckw, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

>     bpf, x86: Add unconditional byte swap to the i386 JIT
>
>     The i386 JIT rejects unconditional BPF byte-swap instructions, which use
>     BPF_ALU64 | BPF_END.
>
>     Route them through the existing big-endian conversion emitter. On
>     little-endian i386, it provides the same 16-, 32- and 64-bit byte swaps.
>     The verifier treats sub-64-bit BPF_END results as 32-bit definitions and
>     inserts zero extension where the upper word is used. The emitter clears
>     that word itself when verifier_zext is disabled.
>
>     Assisted-by: Codex:gpt-6
>     Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>

This looks like a bug fix for the i386 JIT's byte-swap support.  Should this
include:

  Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32")


---
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/34921540254

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-09-15  3:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  2:13 [PATCH bpf-next 0/5] bpf, x86: Extend CPUv4 instruction support in the i386 JIT Nicholas Dudar
2026-09-15  2:13 ` [PATCH bpf-next 1/5] bpf, x86: Add support for BPF_MOVSX " Nicholas Dudar
2026-09-15  3:12   ` bot+bpf-ci
2026-09-15  2:13 ` [PATCH bpf-next 2/5] bpf, x86: Extract the i386 JIT load emitter Nicholas Dudar
2026-09-15  3:12   ` bot+bpf-ci
2026-09-15  2:13 ` [PATCH bpf-next 3/5] bpf, x86: Add BPF_MEMSX support to the i386 JIT Nicholas Dudar
2026-09-15  2:13 ` [PATCH bpf-next 4/5] bpf, x86: Add BPF_JMP32 | BPF_JA " Nicholas Dudar
2026-09-15  3:12   ` bot+bpf-ci
2026-09-15  2:13 ` [PATCH bpf-next 5/5] bpf, x86: Add unconditional byte swap " Nicholas Dudar
2026-09-15  3:12   ` bot+bpf-ci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®