bpf, x86: Normalize IA32 kfunc return values

[kernel-patches-daemon-bpf]

Pull request for series with
subject: bpf, x86: Normalize IA32 kfunc return values
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411

[kernel-patches-daemon-bpf]

Upstream branch: 6f03361
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-review-bot]

commit 0231085a3eba64efc7dc840f67e949f3319224a2
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

bpf, x86: Normalize narrow kfunc returns on IA32

This patch fixes IA32 kfunc return value handling for narrow types
(1/2/4 bytes) by extending them into EDX:EAX and storing both words
of R0, instead of leaving R0.high32 uninitialized.

> 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

In-Reply-To-Subject: bpf, x86: Normalize narrow kfunc returns on IA32
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31699378511

[kernel-patches-daemon-bpf]

Forwarding comment 5280721975 via email
In-Reply-To: 20260813-bpf-next-039-ia32-kfunc-return-v1-v1-1-37495ec632b5@mails.tsinghua.edu.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260813-bpf-next-039-ia32-kfunc-return-v1-v1-1-37495ec632b5@mails.tsinghua.edu.cn/

[kernel-patches-review-bot]

commit b5ed44ff67a8c4f7f28e9d6944f2e8938a17a559
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

selftests/bpf: Cover IA32 narrow kfunc returns

This commit teaches test_loader to select IA32-specific test cases and adds
tests for unsigned bool and signed int kfunc return value handling.

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 8113fea7ba86c..4638e0a45db6e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -127,6 +127,7 @@
>  #include "verifier_set_retval.skel.h"
>  #include "verifier_lsm.skel.h"
>  #include "verifier_jit_inline.skel.h"
> +#include "verifier_kfunc_return.skel.h"
>  #include "irq.skel.h"
>  #include "verifier_ctx_ptr_param.skel.h"
>  #include "verifier_zext.skel.h"
> @@ -289,6 +290,7 @@ void test_verifier_lsm(void)                  { RUN(verifier_lsm); }
>  void test_irq(void)			      { RUN(irq); }
>  void test_verifier_mtu(void)		      { RUN(verifier_mtu); }
>  void test_verifier_jit_inline(void)               { RUN(verifier_jit_inline); }
> +void test_verifier_kfunc_return(void)             { RUN(verifier_kfunc_return); }
>  void test_verifier_ctx_ptr_param(void)       { RUN(verifier_ctx_ptr_param); }
>  void test_verifier_zext(void)                 { RUN_TESTS(verifier_zext); }

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_kfunc_return.c b/tools/testing/selftests/bpf/progs/verifier_kfunc_return.c
> new file mode 100644
> index 0000000000000..52a94daf03922
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_kfunc_return.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define BPF_NO_KFUNC_PROTOTYPES
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +extern int bpf_strcmp(const char *s1__ign, const char *s2__ign) __ksym;
> +extern bool bpf_dynptr_is_null(const struct bpf_dynptr *p) __ksym;
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} kfunc_return_map SEC(".maps");
> +
> +static const char string_a[] = "a";
> +static const char string_b[] = "b";
> +
> +void __kfunc_btf_root(void)
> +{
> +	struct bpf_dynptr ptr = {};
> +
> +	bpf_strcmp(string_a, string_b);
> +	bpf_dynptr_is_null(&ptr);
> +}
> +
> +SEC("syscall")
> +__flag(BPF_F_SLEEPABLE)
> +__success __retval(0)
> +__arch_x86_32
> +__naked void kfunc_bool_return_zero_extended(void)
> +{
> +	asm volatile ("					\
> +	r0 = 0;						\
> +	*(u32 *)(r10 - 4) = r0;				\
> +	r2 = r10;					\
> +	r2 += -4;					\
> +	r1 = %[kfunc_return_map] ll;			\
> +	call %[bpf_map_lookup_elem];			\
> +	if r0 == 0 goto 1f;				\
> +	r1 = r0;					\
> +	r2 = 8;						\
> +	r3 = 0;						\
> +	r4 = r10;					\
> +	r4 += -24;					\
> +	call %[bpf_dynptr_from_mem];			\
> +	if r0 != 0 goto 1f;				\
> +	r0 = 0x100000000 ll;				\
> +	r1 = r10;					\
> +	r1 += -24;					\
> +	call bpf_dynptr_is_null;				\
> +	r0 >>= 32;					\
> +	exit;						\
> +1:	r0 = 2;						\
> +	exit;						\

Can this test actually detect incorrect sign extension? The dynptr is
initialized by a successful bpf_dynptr_from_mem() on a non-NULL map value,
so bpf_dynptr_is_null() always returns false. Zero- and sign-extension of
0 are identical, so 'r0 >>= 32' followed by 'exit' yields 0 whether the
JIT emits movzx or movsx.

A JIT that wrongly sign-extended an unsigned/bool narrow return would
still pass this test. Reaching a true value would make the case
discriminating - for example, bpf_dynptr_is_rdonly() on a read-only
dynptr returns 1, or an unsigned kfunc that returns a value with bit 31
set.

The commit message states that 'each case ... checks the required zero or
sign extension', but that claim only holds for the int case below.

The new switch on fm->ret_size in emit_kfunc_call() has six size by
signedness arms plus a 'default: return -EINVAL', but this patch only
exercises two of them (bool/1-byte unsigned and 4-byte signed).

> +
> +SEC("syscall")
> +__flag(BPF_F_SLEEPABLE)
> +__success __retval(-1)
> +__arch_x86_32
> +__naked void kfunc_int_return_sign_extended(void)
> +{
> +	asm volatile ("					\
> +	r0 = 0;						\
> +	r1 = %[string_a] ll;				\
> +	r2 = %[string_b] ll;				\
> +	call bpf_strcmp;				\
> +	r0 >>= 32;					\
> +	exit;						\

Does this test codify an architecture-specific extension behavior that
conflicts with the documented ISA and other JITs? The test asserts that
after 'call bpf_strcmp' (which returns int -1 for "a" vs "b"),
'r0 >>= 32' yields 0xffffffff - meaning it expects sign-extension of a
narrow signed kfunc return into the upper 32 bits of R0.

The verifier deliberately does not model any extension after a kfunc call.
Looking at bpf_opt_subreg_zext_lo32_rnd_hi32() in kernel/bpf/fixups.c,
it skips kfunc calls entirely:

    if (!bpf_jit_needs_zext() && !is_cmpxchg_insn(&insn))
            continue;
    /* Zero-extension is done by the caller. */
    if (bpf_pseudo_kfunc_call(&insn))
            continue;

Everywhere else in the ISA a 32-bit definition read as 64 bits is
zero-extended, so the documented fallback contract for a narrow kfunc
return is zero-extension.

On x86-64 the identical program returns 0, because a callee returning int
leaves the upper half of RAX unspecified-but-in-practice-zero and the JIT
stores RAX as-is. No other JIT normalizes narrow kfunc returns either:
s390 extends kfunc arguments but emits a plain 'lgr %b0,%r2' for the
return; riscv64 and loongarch only extend returns in the trampoline and
struct_ops paths.

So the pair of commit 0231085a3eba6 plus this test locks IA32 into
sign-extending signed narrow kfunc returns while every other target
zero-extends, and encodes that divergence as a passing selftest. Either
the sign-extension convention should be stated as the cross-arch contract
(and the other JITs plus the fixups.c comment updated, with the test
tagged for all arches), or IA32 should zero-extend like the rest and this
case should expect __retval(0).

As written, a future patch that makes IA32 match x86-64 will be reported
as a selftest regression.

> +
> +char _license[] SEC("license") = "GPL";

A subsystem pattern flags this as potentially concerning: a whole new
prog file (84 lines) plus a new prog_tests/verifier.c entry point is
added for two cases about the upper 32 bits of R0 after a kfunc call.

Looking at tools/testing/selftests/bpf/progs/verifier_zext.c in the same
directory, it is already the home for 32-bit/64-bit register-half
semantics and contains exactly the same scaffolding this new file
re-creates: BPF_NO_KFUNC_PROTOTYPES, an extern __ksym declaration, the
__kfunc_btf_root() BTF-retention idiom, and __naked asm that calls a
kfunc by name.

Would adding the two subtests to verifier_zext.c avoid the duplicated
boilerplate, the extra skeleton, and the extra test entry point? The new
entry point also uses RUN() (which drops CAP_SYS_ADMIN) while the sibling
test_verifier_zext() uses RUN_TESTS() - another indication the two might
belong together.

> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> index 07807757b518d..8a9f4a31a1bb1 100644
> --- a/tools/testing/selftests/bpf/test_loader.c
> +++ b/tools/testing/selftests/bpf/test_loader.c
> @@ -374,16 +374,19 @@ static const char **collect_decl_tags(struct btf *btf, int id, int cnt)
>  enum arch {
>  	ARCH_UNKNOWN	= 0x1,
>  	ARCH_X86_64	= 0x2,
> -	ARCH_ARM64	= 0x4,
> -	ARCH_RISCV64	= 0x8,
> -	ARCH_S390X	= 0x10,
> -	ARCH_LOONGARCH	= 0x20,
> +	ARCH_X86_32	= 0x4,
> +	ARCH_ARM64	= 0x8,
> +	ARCH_RISCV64	= 0x10,
> +	ARCH_S390X	= 0x20,
> +	ARCH_LOONGARCH	= 0x40,
>  };
>
>  static int get_current_arch(void)
>  {
>  #if defined(__x86_64__)
>  	return ARCH_X86_64;
> +#elif defined(__i386__)
> +	return ARCH_X86_32;
>  #elif defined(__aarch64__)
>  	return ARCH_ARM64;
>  #elif defined(__riscv) && __riscv_xlen == 64

Can ARCH_X86_32 mismatch the actual JIT when test_progs is cross-compiled?
ARCH_X86_32 is derived from the compile-time architecture of the
test_progs userspace binary, but the new __arch_x86_32 cases assert a
retval that depends on the kernel's JIT.

For every pre-existing __arch_* user this distinction does not matter,
because those tags only gate JIT-disassembly matching and a 64-bit
test_progs cannot run on a 32-bit kernel.

The reverse combination is legitimate and common though: a 32-bit (i386)
test_progs running on an x86-64 kernel. There get_current_arch() returns
ARCH_X86_32, run_subtest() does not skip, and both new subtests execute
against the x86-64 JIT, which does not sign-extend the bpf_strcmp()
return.

In that scenario kfunc_int_return_sign_extended fails with 'Unexpected
retval: 0 != -1'. If the intent is to detect when the kernel uses the
IA32 JIT, would the gate need a kernel-side probe (such as uname machine,
or a JIT-behaviour probe) rather than __i386__, or would the new cases
need a DENYLIST entry?

> @@ -577,6 +580,8 @@ static int parse_test_spec(struct test_loader *tester,
>  		} else if ((val = str_has_pfx(s, "test_arch="))) {
>  			if (strcmp(val, "X86_64") == 0) {
>  				arch = ARCH_X86_64;
> +			} else if (strcmp(val, "X86_32") == 0) {
> +				arch = ARCH_X86_32;
>  			} else if (strcmp(val, "ARM64") == 0) {
>  				arch = ARCH_ARM64;
>  			} else if (strcmp(val, "RISCV64") == 0) {

---

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

In-Reply-To-Subject: selftests/bpf: Cover IA32 narrow kfunc returns
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31699378511

[kernel-patches-daemon-bpf]

Forwarding comment 5280798949 via email
In-Reply-To: 20260813-bpf-next-039-ia32-kfunc-return-v1-v1-2-37495ec632b5@mails.tsinghua.edu.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260813-bpf-next-039-ia32-kfunc-return-v1-v1-2-37495ec632b5@mails.tsinghua.edu.cn/

[kernel-patches-daemon-bpf]

Upstream branch: 9786d42
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 806c1a1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: aacd13e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 259d60f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 4d9551b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: adb7719
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 6b0835a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 6ab6a94
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 3d9393f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: f79066c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 91ec203
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: aed1bf1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: d19862d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 4191e49
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 4954de7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: e40f61a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 669e4fa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: a284ed4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: d83fba2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: ce36e38
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 05ea1b6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 1555de3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145411
version: 1