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

On 5/9/26 4:59 AM, [email protected] wrote:
> 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 <[email protected]>
>
> 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?

This is a new feature. So it won't cause regression from this patch
set perspective. But indeed, r11 based insns are enabled, but jit is
not enabled, but this is not 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).

No. jit is not enabled yet for stack arguments. Also for the below
example.

>
> 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?

The verifier should be able to prevent that. See previous comments
in "bpf: Reject stack arguments in non-JITed programs".

>
> 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?

I guess it should be okay. It won't break the kernel although
verificaiton will failure.

>
>>   			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;
>>   		}