Re: [PATCH bpf-next v5 2/9] bpf: Introduce global percpu data

[Leon Hwang <leon.hwang@linux.dev>]

On 8/6/26 23:13, sashiko-bot@kernel.org wrote:
[...]
>> +
>> +			aux = &env->insn_aux_data[i + delta];
>> +			map = env->used_maps[aux->map_index];
>> +			if (map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY)
>> +				goto next_insn;
>> +
>> +			/*
>> +			 * Reuse the original ld_imm64 insn, and add one
>> +			 * mov64_percpu_reg insn.
>> +			 */
>> +
>> +			insn_buf[0] = insn[1];
>> +			insn_buf[1] = BPF_MOV64_PERCPU_REG(insn->dst_reg, insn->dst_reg);
> 
> [Severity: Critical]
> Can this crash the kernel if JIT compilation fails dynamically and falls back
> to the BPF interpreter?


Sounds reasonable.

I think it is a pre-existing issue, because the interpreter does not
support BPF_MOV64_PERCPU_REG() insn.

Also, the interpreter does not support user addr_space_cast insn.

Will fix them btw.

> 
> While env->prog->jit_requested checks if JIT was requested, it doesn't
> guarantee JIT success. If the interpreter (___bpf_prog_run()) executes this
> BPF_MOV64_PERCPU_REG instruction, it ignores the special BPF_ADDR_PERCPU
> offset (-1) for ALU64_MOV_X:
> 
[...]> kernel/bpf/verifier.c:check_mem_access() {
> ...
> 			if (tnum_is_const(reg->var_off) &&
> 			    bpf_map_is_rdonly(map) &&
> 			    map->ops->map_direct_value_addr &&
> 			    map->map_type != BPF_MAP_TYPE_INSN_ARRAY) {
> ...
> }
> 
> returning -EINVAL here propagates the error back up to check_mem_access()
> and aborts verification.
> 

Correct.

I've added a test to verify it that it will return -EINVAL for rdonly
percpu_array map.

Will exclude BPF_MAP_TYPE_PERCPU_ARRAY here, and add a test.

Thanks,
Leon

>>  	err = map->ops->map_direct_value_addr(map, &addr, off);
>>  	if (err)
>>  		return err;
>