Re: [PATCH bpf-next v10 14/24] bpf, x86: Dispatch exception cleanup pads at run time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: Exception unwinder cleanup pad executes in caller's stack frame context
--

commit 9d03eddc4bc6be397e72d866390a80929337af59
Author: Yonghong Song <yonghong.song@linux.dev>

bpf, x86: Dispatch exception cleanup pads at run time

Add what bpf_unwind() relies on to send each frame of a program to its
landing pad or its epilogue on x86-64. This includes arch_bpf_stack_walk_ra(),
aux->epilogue_ip assignment, and bpf_exc_fill_native_ranges() call.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 083fcd6cf15b7..d434871b5a11c 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> @@ -3279,6 +3280,8 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
>  			seen_exit = true;
>  			/* Update cleanup_addr */
>  			ctx->cleanup_addr = proglen;
> +			/* Where an unwind sends a frame with no pad. */
> +			bpf_prog->aux->epilogue_ip = (u64)image + proglen;
>  			if (bpf_prog_was_classic(bpf_prog) &&
>  			    !ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)) {
>  				if (emit_spectre_bhb_barrier(&prog, ip, bpf_prog))
>  					return -EINVAL;
>  			}
>  			/* Deallocate outgoing args 7+ area. */
>  			emit_add_rsp(&prog, outgoing_rsp);
>  			if (bpf_prog->aux->exception_boundary) {
>  				pop_callee_regs(&prog, all_callee_regs_used);
>  				pop_r12(&prog);
>  			} else {
>  				pop_callee_regs(&prog, callee_regs_used);
>  				if (arena_vm_start)
>  					pop_r12(&prog);
>  			}
>  			EMIT1(0xC9);         /* leave */
>  			bpf_prog->aux->ksym.fp_end = prog - temp;
>  
>  			emit_return(&prog, image + addrs[i - 1] + (prog - temp));

[Severity: High]
Does this execute the cleanup pad in the wrong stack frame context?

During exception unwinding, arch_bpf_stack_walk_ra() walks the stack and
yields the return address pointer (ra) for the current frame N, which
points to the instruction in frame N-1.

In bpf_unwind_rewrite(), *ra is overwritten with the address of frame N's
cleanup pad or epilogue_ip:

kernel/bpf/helpers.c:bpf_unwind_rewrite() {
    ...
    rec = bpf_exc_pad_for_ip(prog, ip);
    *ra = rec ? rec->pad : prog->aux->epilogue_ip;
    ...
}

This forces frame N to jump to its cleanup pad when it returns.

However, the JIT epilogue executes "leave" before "ret". The "leave"
instruction completely tears down frame N's stack and restores frame N-1's
stack pointer and frame pointer.

The "ret" instruction then pops the modified return address and jumps
to frame N's pad. As a result, the pad executes while frame N-1's context
is active.

Can this cause any stack accesses inside the pad (using rbp, mapped to r10
in BPF) to incorrectly access frame N-1's stack variables, leading to
memory corruption, use-after-free, or garbage reads during cleanup?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009041447.17092-1-yonghong.song@linux.dev?part=14