Re: [PATCH bpf] bpf, x86: Fix signed 32-bit div/mod in the i386 JIT

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Nicholas,

On Mon, Jul 13, 2026 at 02:58:48PM -0400, Nicholas Dudar wrote:
> emit_ia32_div_mod_r() always emits an unsigned divide (xor edx,edx +
> div ecx), regardless of the BPF instruction's signedness. Signed
> ALU32 BPF_DIV and BPF_MOD (off=1) therefore get an unsigned quotient
> and remainder in the i386 JIT, diverging from the verifier's and the
> interpreter's signed result for negative operands.
> 
> Thread is_signed = (insn->off == 1) into emit_ia32_div_mod_r() and
> emit cdq + idiv ecx on the signed path, xor edx,edx + div ecx
> unchanged on the unsigned path, mirroring the cdq/idiv-vs-xor/div
> split the x86_64 JIT uses for the same instructions. bpf_do_misc_fixups()
> rewrites the zero-divisor and INT_MIN/-1 cases out of the instruction
> stream before the JIT runs.
> 
> The RV32 JIT has the same gap.
> 
> Fixes: ec0e2da95f72 ("bpf: Support new signed div/mod instructions.")

As suggested by Pu Lehui [1], let's use 'Add support for xxx' and drop
the Fixes tag.

[1]: https://lore.kernel.org/bpf/fa8a040f-64c4-484f-9538-fb0ec287639f@huawei.com/

> Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> ---
>  arch/x86/net/bpf_jit_comp32.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 852baf2e4db4..46b47432f50a 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -433,7 +433,8 @@ static inline void emit_ia32_to_be_r64(const u8 dst[], s32 val,
>   * dst = dst (div|mod) src
>   */
>  static inline void emit_ia32_div_mod_r(const u8 op, const u8 dst, const u8 src,
> -				       bool dstk, bool sstk, u8 **pprog)
> +				       bool dstk, bool sstk, bool is_signed,
> +				       u8 **pprog)

Let's combine op and is_signed to struct bpf_insn *insn to simplify the
parameters.

Regards,
Kuan-Wei

>  {
>  	u8 *prog = *pprog;
>  	int cnt = 0;
> @@ -454,10 +455,17 @@ static inline void emit_ia32_div_mod_r(const u8 op, const u8 dst, const u8 src,
>  		/* mov eax,dst */
>  		EMIT2(0x8B, add_2reg(0xC0, dst, IA32_EAX));
>  
> -	/* xor edx,edx */
> -	EMIT2(0x31, add_2reg(0xC0, IA32_EDX, IA32_EDX));
> -	/* div ecx */
> -	EMIT2(0xF7, add_1reg(0xF0, IA32_ECX));
> +	if (is_signed) {
> +		/* cdq */
> +		EMIT1(0x99);
> +		/* idiv ecx */
> +		EMIT2(0xF7, add_1reg(0xF8, IA32_ECX));
> +	} else {
> +		/* xor edx,edx */
> +		EMIT2(0x31, add_2reg(0xC0, IA32_EDX, IA32_EDX));
> +		/* div ecx */
> +		EMIT2(0xF7, add_1reg(0xF0, IA32_ECX));
> +	}
>  
>  	if (op == BPF_MOD) {
>  		if (dstk)
> @@ -1796,7 +1804,8 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
>  			switch (BPF_SRC(code)) {
>  			case BPF_X:
>  				emit_ia32_div_mod_r(BPF_OP(code), dst_lo,
> -						    src_lo, dstk, sstk, &prog);
> +						    src_lo, dstk, sstk,
> +						    insn->off == 1, &prog);
>  				break;
>  			case BPF_K:
>  				/* mov ecx,imm32*/
> @@ -1804,7 +1813,7 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
>  					    imm32);
>  				emit_ia32_div_mod_r(BPF_OP(code), dst_lo,
>  						    IA32_ECX, dstk, false,
> -						    &prog);
> +						    insn->off == 1, &prog);
>  				break;
>  			}
>  			if (!bpf_prog->aux->verifier_zext)
> 
> base-commit: 7cbd0c4cebe4c9f678d15e6b9ba975e1155a107f
> -- 
> 2.34.1
>