Re: [PATCH bpf-next v2 11/20] bpf, x86: Dispatch exception cleanup pads at run time

[Yonghong Song <yonghong.song@linux.dev>]

On 9/17/26 10:44 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/arch/x86/net/Makefile b/arch/x86/net/Makefile
>> index dddbefc0f4398..9d574d972df37 100644
>> --- a/arch/x86/net/Makefile
>> +++ b/arch/x86/net/Makefile
>> @@ -6,5 +6,5 @@
>>   ifeq ($(CONFIG_X86_32),y)
>>           obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o
>>   else
>> -        obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o bpf_timed_may_goto.o
>> +        obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o bpf_timed_may_goto.o bpf_cleanup_pad.o
>>   endif
>> diff --git a/arch/x86/net/bpf_cleanup_pad.S b/arch/x86/net/bpf_cleanup_pad.S
>> new file mode 100644
>> index 0000000000000..da4b448ecf095
>> --- /dev/null
>> +++ b/arch/x86/net/bpf_cleanup_pad.S
> [ ... ]
>
>> +SYM_FUNC_START(arch_bpf_run_cleanup_pad)
>> +	ANNOTATE_NOENDBR
>> +
>> +	pushq %rbp
>> +	pushq %rbx
>> +	pushq %r12
>> +	pushq %r13
>> +	pushq %r14
>> +	pushq %r15
>> +	/* Keep the pad's entry rsp congruent to a normal call's. */
>> +	subq $8, %rsp
>> +
>> +	movq 0(%rdx), %r15
>> +	movq 8(%rdx), %r14
>> +	movq 16(%rdx), %r13
>> +	movq 24(%rdx), %rbx
>> +	movq 32(%rdx), %r12
>> +	/* rbp is BPF r10, so this is the whole of the pad's frame setup. */
>> +	movq %rsi, %rbp
>> +
>> +	/* CALL_NOSPEC needs the target in a register; rcx is BPF r4, dead. */
>> +	movq %rdi, %rcx
>> +
>> +	/* BPF r0 on the way into a pad, not whatever the kernel left in rax. */
>> +	movl $BPF_PAD_ENTRY_R0, %eax
>> +
>> +	CALL_NOSPEC rcx
>> +
>> +	addq $8, %rsp
>> +	popq %r15
>> +	popq %r14
>> +	popq %r13
>> +	popq %r12
>> +	popq %rbx
>> +	popq %rbp
>> +	RET
>> +SYM_FUNC_END(arch_bpf_run_cleanup_pad)
> Will this build with CONFIG_STACK_VALIDATION enabled?
>
> arch_bpf_run_cleanup_pad() is declared with SYM_FUNC_START (an STT_FUNC
> symbol) and saves rbp with 'pushq %rbp', but never establishes a frame
> pointer with 'mov %rsp, %rbp'. Instead, it overwrites rbp with the BPF
> frame pointer (movq %rsi, %rbp) and then makes a call (CALL_NOSPEC rcx).
>
> objtool's frame-pointer validation (CONFIG_STACK_VALIDATION with
> UNWINDER_FRAME_POINTER) checks for two conditions in validate_call()
> and validate_return():
>
> tools/objtool/check.c:has_valid_stack_frame() requires cfi->cfa.base ==
> CFI_BP, which is only set when the function does 'mov %rsp, %rbp' to
> establish a frame. This function never does that, so the CALL_NOSPEC
> would trigger:
>
>      WARN_INSN(insn, "call without frame pointer save/setup");
>
> Additionally, the 'pushq %rbp' while cfa->base != CFI_BP sets
> cfi->bp_scratch = true, which is only cleared by 'mov %rsp, %rbp'.
> This flag is still set at the RET, where validate_return() warns:
>
>      WARN_INSN(insn, "BP used as a scratch register");
>
> The Makefile adds bpf_cleanup_pad.o unconditionally under
> obj-$(CONFIG_BPF_JIT), even though the feature is gated on
> CONFIG_UNWINDER_ORC per bpf_jit_supports_cleanup_pads(). This means
> in a CONFIG_STACK_VALIDATION=y build (frame-pointer unwinder), the
> object would fail objtool validation, and with CONFIG_OBJTOOL_WERROR=y
> the build would fail.
>
> The neighbouring arch/x86/net/bpf_timed_may_goto.S does not have this
> issue because it sets up a proper frame:
>
> arch/x86/net/bpf_timed_may_goto.S:
>      pushq %rbp
>      movq %rsp, %rbp
>      ...
>      leave
>
> rbp cannot be the frame pointer here because it must carry BPF r10 into
> the pad. This is the same situation arch/x86/kernel/ftrace_64.S handles
> with STACK_FRAME_NON_STANDARD_FP(ftrace_caller). Could either
> STACK_FRAME_NON_STANDARD_FP(arch_bpf_run_cleanup_pad) after SYM_FUNC_END
> or building the object only when CONFIG_UNWINDER_ORC is set (matching the
> feature gate) address this?

Yes, jit only supports CONFIG_UNWINDER_ORC for x86, and CONFIG_STACK_VALIDATION
requires CONFIG_UNWINDER_FRAME_POINTER.

>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index d4a980140b48d..9d0dd54773e83 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -357,6 +357,11 @@ struct jit_context {
>>   /* Number of bytes that will be skipped on tailcall */
>>   #define X86_TAIL_CALL_OFFSET	(12 + ENDBR_INSN_SIZE)
>>
>> +/* Throw-site spill: r15, r14, r13, rbx, r12 low to high, the layout the
>> + * prologue's pushes leave, so arch_bpf_run_cleanup_pad() reads both alike.
>> + */
>> +#define X86_CLEANUP_SPILL_SZ	(5 * 8)
> This isn't a bug, but could the comment format be updated to match the
> BPF subsystem style? The BPF subsystem guide requires the modern kernel
> multi-line comment style, where the opening /* is on its own line. The
> patch already uses this format for its other new multi-line comments
> (the "Lowest address of each spill area" block and the "Spill r6-r9 and
> r12" block later in do_jit), so is there a reason for the inconsistency
> here?

Will adjust comment.

>
> [ ... ]
>
>
> ---
> 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/35308528711