> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 852baf2e4db4d..017cc9e7e4e2b 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
[ ... ]
> @@ -1620,13 +1620,25 @@ static int emit_kfunc_call(const struct bpf_prog
> *bpf_prog, u8 *end_addr,
> /* add esp,"bytes_in_stack" */
> end_addr -= 3;
>
> - /* mov dword ptr [ebp+off],edx */
> - if (fm->ret_size > sizeof(u32))
> - end_addr -= 3;
> -
> - /* mov dword ptr [ebp+off],eax */
> - if (fm->ret_size)
> + switch (fm->ret_size) {
> + case 0:
> + break;
> + case sizeof(u8):
> + case sizeof(u16):
> + /* mov[s|z]x eax,[a]x */
> end_addr -= 3;
> + fallthrough;
> + case sizeof(u32):
> + /* cdq or xor edx,edx */
> + end_addr -= fm->ret_flags & BTF_FMODEL_SIGNED_ARG ? 1 : 2;
> + fallthrough;
> + case sizeof(u64):
> + /* Store EDX:EAX in R0. */
> + end_addr -= 6;
> + break;
> + default:
> + return -EINVAL;
> + }
The comment above this diff hunk reads:
* address by figuring out how many jit-insn is generated between
* the call (0xE8) and the end_addr:
* - 0-1 jit-insn (3 bytes each) to restore the esp pointer if there
* is arg pushed to the stack.
* - 0-2 jit-insns (3 bytes each) to handle the return value.
Should this comment be updated? The new code can emit 0-4 instructions
with varying lengths (1, 2, 3 or 6 bytes) to extend and store the
return value, not just "0-2 jit-insns (3 bytes each)".
>
> jmp_offset = (u8 *)__bpf_call_base + insn->imm - end_addr;
> if (!is_simm32(jmp_offset)) {
> @@ -1637,15 +1649,38 @@ static int emit_kfunc_call(const struct bpf_prog
> *bpf_prog, u8 *end_addr,
>
> EMIT1_off32(0xE8, jmp_offset);
>
> - if (fm->ret_size)
> + switch (fm->ret_size) {
> + case sizeof(u8):
> + /* mov[s|z]x eax,al */
> + EMIT3(0x0F, fm->ret_flags & BTF_FMODEL_SIGNED_ARG ? 0xBE : 0xB6,
> + add_2reg(0xC0, IA32_EAX, IA32_EAX));
> + break;
> + case sizeof(u16):
> + /* mov[s|z]x eax,ax */
> + EMIT3(0x0F, fm->ret_flags & BTF_FMODEL_SIGNED_ARG ? 0xBF : 0xB7,
> + add_2reg(0xC0, IA32_EAX, IA32_EAX));
> + break;
> + default:
> + break;
> + }
> +
> + if (fm->ret_size && fm->ret_size <= sizeof(u32)) {
> + if (fm->ret_flags & BTF_FMODEL_SIGNED_ARG)
> + /* cdq */
> + EMIT1(0x99);
> + else
> + /* xor edx,edx */
> + EMIT2(0x31, add_2reg(0xC0, IA32_EDX, IA32_EDX));
> + }
Does sign-extending narrow signed returns match the documented kfunc
return contract? In kernel/bpf/fixups.c the comment says:
/* Zero-extension is done by the caller. */
if (bpf_pseudo_kfunc_call(&insn))
continue;
This exempts kfunc calls from the usual zero-extension path because
the JIT is responsible for R0's high half. The verifier's
__mark_btf_func_reg_size() marks narrow returns as subregs
specifically to invoke the zero-extension machinery.
For a concrete example with bpf_strcmp() which returns signed int,
if it returns -1:
- x86-64 leaves R0 = 0x00000000ffffffff (32-bit write zeros upper half)
- This patch makes x86-32 produce R0 = 0xffffffffffffffff via cdq
The same BPF program would see different R0 values depending on the
architecture. Is this the intended behavior, or should narrow kfunc
returns be zero-extended unconditionally to match the x86-64 path and
the fixups.c contract?
> +
> + if (fm->ret_size) {
> /* mov dword ptr [ebp+off],eax */
> EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EAX),
> STACK_VAR(bpf2ia32[BPF_REG_0][0]));
> -
> - if (fm->ret_size > sizeof(u32))
> /* mov dword ptr [ebp+off],edx */
> EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
> STACK_VAR(bpf2ia32[BPF_REG_0][1]));
> + }
[ ... ]
---
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/31699378511
