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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Interpreter fallback vulnerability due to missing BPF_MOV64_PERCPU_REG support
--

commit a68c25810c5f3ada3bbae32b3967070021ca8848
Author: Leon Hwang <leon.hwang@linux.dev>

bpf: Introduce global percpu data

This commit introduces global percpu data support for BPF, allowing
definition of global percpu variables similar to DEFINE_PER_CPU(). This
enables tools to simplify their BPF code by avoiding percpu_array maps
and manual CPU ID lookups.

Links:
[1] https://github.com/anakryiko/retsnoop
[2] https://github.com/bpfsnoop/bpfsnoop

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 3cf2cc6e3ab66..4f84d087ca69e 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
[ ... ]
> @@ -1819,6 +1819,38 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
>  			goto next_insn;
>  		}
>  
> +		if (env->prog->jit_requested &&
> +		    bpf_jit_supports_percpu_insn() &&
> +		    insn->code == (BPF_LD | BPF_IMM | BPF_DW) &&
> +		    (insn->src_reg == BPF_PSEUDO_MAP_VALUE ||
> +		     insn->src_reg == BPF_PSEUDO_MAP_IDX_VALUE)) {
> +			struct bpf_map *map;
> +
> +			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: High]
This is a pre-existing issue, but does the BPF interpreter properly handle
BPF_MOV64_PERCPU_REG?

The verifier correctly checks bpf_jit_supports_percpu_insn() before inserting
this instruction. However, if JIT compilation fails later (e.g., due to memory
limits) and the system falls back to the interpreter, what happens to this
instruction?

Looking at ___bpf_prog_run() in kernel/bpf/core.c:

    ALU64_MOV_X:
        switch (OFF) {
        case 0:
            DST = SRC;
            break;

The interpreter appears to silently ignore OFF == BPF_ADDR_PERCPU, essentially
treating it as a no-op or standard move.

Would this leak the raw __percpu pointer (which is often a small integer
offset on architectures like x86) into subsequent memory access instructions?
While this patch didn't introduce the interpreter fallback behavior, it seems
to expand the surface area by allowing direct read/write accesses to arbitrary
percpu arrays. Could this lead to out-of-bounds memory accesses if JIT fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629152406.52582-1-leon.hwang@linux.dev?part=2