Re: [PATCH v5 07/13] bpf, x86: Take a Tasks Trace reader in the trampoline around its call-outs

[bot+bpf-ci]

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index d4a980140b48..93743628b0fa 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -3881,6 +3967,16 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>  	/* mov QWORD PTR [rbp - rbx_off], rbx */
>  	emit_stx(&prog, BPF_REG_FP, BPF_REG_6, -rbx_off);
>
> +	/*
> +	 * Tasks RCU keeps this image alive only while we are a Tasks Trace
> +	 * reader; the instructions before this point (and after the final
> +	 * unlock) are covered by the irq-exit IP check.  One reader spans
> +	 * __bpf_tramp_enter() and the fentry/fmod_ret progs, a second one
> +	 * the fexit progs and __bpf_tramp_exit(); the original function runs
> +	 * outside both, with the image pinned by im->pcref instead.
> +	 */
> +	emit_trace_rcu_reader(&prog, true);
> +

Does this take the Tasks Trace reader too late for BPF_TRAMP_F_INDIRECT
trampolines with FineIBT+BHI mitigation?

Looking back at the code around line 3943 (before this reader lock):

    if (flags & BPF_TRAMP_F_INDIRECT) {
        /*
         * Indirect call for bpf_struct_ops
         */
        emit_cfi(&prog, image,
                 cfi_get_func_hash(func_addr),
                 cfi_get_func_arity(func_addr));
    } else {
        x86_call_depth_emit_accounting(&prog, NULL, image);
    }

When CONFIG_FINEIBT_BHI is enabled, emit_cfi() dispatches to
emit_fineibt(), which emits:

    EMIT_ENDBR();
    EMIT1_off32(0x2d, hash);            /* sub $hash, %eax */
    if (cfi_bhi) {
        EMIT2(0x2e, 0x2e);              /* cs cs */
        emit_call(&prog, __bhi_args[arity], ip + 11);
    }

So the very first instructions in the trampoline (around offset 9,
before the reader is taken at line 3978) include a call out of the
image into __bhi_args[arity].

__bhi_args lives in .noinstr.text (arch/x86/lib/bhi.S), which is core
kernel text. It's not covered by arch_rcu_tasks_trampoline_text()
(arch/x86/kernel/ftrace.c covers only ftrace_caller ranges,
__return_thunk ranges, and __rethunk_safe ranges).

If a task is preempted (irq-exit reschedule) inside __bhi_args while its
return address points back into the trampoline image, it is neither
holding the Tasks Trace reader nor recognized by the IP check. Can
bpf_struct_ops_map_free() then complete its synchronize_rcu_tasks() and
free the image via arch_free_bpf_trampoline() before the task resumes
and returns into freed memory?

Would moving emit_trace_rcu_reader() before the emit_cfi() call close
this window, or would adding the __bhi_args range to
arch_rcu_tasks_trampoline_text() be safer?


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