Re: [PATCH bpf-next v2 12/23] bpf: Enable r11 based insns

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch enables the use of `BPF_REG_PARAMS` (`r11`) in BPF instructions but fails to include the corresponding JIT compiler and interpreter support, allowing an attacker to achieve arbitrary kernel memory reads and writes.
--

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

bpf: Enable r11 based insns

This patch enables the use of BPF_REG_PARAMS (r11) for stack argument
accesses in the BPF verifier. It updates check_and_resolve_insns() to
accept ldx, st, and stx instructions that use r11 if they match the
stack argument formats.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4827df9f817c2..e208807e79959 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18004,11 +18004,12 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
>  		return err;
>  
>  	for (i = 0; i < insn_cnt; i++, insn++) {
> -		if (insn->dst_reg >= MAX_BPF_REG) {
> +		if (insn->dst_reg >= MAX_BPF_REG &&
> +		    !is_stack_arg_st(insn) && !is_stack_arg_stx(insn)) {

Does enabling r11 here without corresponding JIT and interpreter updates
introduce a regression?

If a program uses r11, but the JIT backends lack a mapping for it, could
it map to an unexpected hardware register? For example, on x86-64, if
reg2hex[11] defaults to 0, it maps to %rax (BPF_REG_0).

Since a BPF program can control the value of BPF_REG_0, an instruction
like BPF_STX [r11 - 8], r1 might be compiled to mov %rdi, -8(%rax). Could
this allow an arbitrary memory write bypassing the verifier sandbox?

In non-JIT environments, could regs[11] in the interpreter be
uninitialized, leading to uninitialized pointer dereferences?

It appears this regression is addressed later in the series by commit
ee1dd512e669a ("bpf,x86: Implement JIT support for stack arguments")
and commit 6ccf042549a39 ("bpf, arm64: Add JIT support for stack
arguments"), but could applying this patch on its own break bisection?

>  			verbose(env, "R%d is invalid\n", insn->dst_reg);
>  			return -EINVAL;
>  		}
> -		if (insn->src_reg >= MAX_BPF_REG) {
> +		if (insn->src_reg >= MAX_BPF_REG && !is_stack_arg_ldx(insn)) {
>  			verbose(env, "R%d is invalid\n", insn->src_reg);
>  			return -EINVAL;
>  		}

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