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

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

> diff --git a/kernel/bpf/const_fold.c b/kernel/bpf/const_fold.c
> index b2a19acadb91..5787246bef30 100644
> --- a/kernel/bpf/const_fold.c
> +++ b/kernel/bpf/const_fold.c
> @@ -182,7 +182,6 @@ static void const_reg_xfer(struct bpf_verifier_env *env, struct const_arg_info *
>  		u64 val = 0;
>
>  		if (!bpf_map_is_rdonly(map) || !map->ops->map_direct_value_addr ||
> -		    map->map_type == BPF_MAP_TYPE_INSN_ARRAY ||
>  		    off < 0 || off + size > map->value_size ||
>  		    bpf_map_direct_read(map, off, size, &val, is_ldsx)) {
>  			*dst = unknown;

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2690d063a240..eecd9ab82e91 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5556,6 +5556,8 @@ int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val,
>  	u64 addr;
>  	int err;
>
> +	if (map->map_type == BPF_MAP_TYPE_INSN_ARRAY || map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> +		return -EINVAL;
>  	err = map->ops->map_direct_value_addr(map, &addr, off);

Here bpf_map_direct_read() now refuses both insn arrays and percpu arrays,
which makes sense since a percpu value address cannot be constant-folded
at verification time.

The const_fold.c hunk above was adjusted to cope with this: it dropped its
own BPF_MAP_TYPE_INSN_ARRAY pre-check and instead lets a bpf_map_direct_read()
failure fall through to "*dst = unknown".

Is the equivalent read path in check_mem_access() missing the same treatment?

kernel/bpf/verifier.c:check_mem_access() {
    ...
    /*
     * If map is read-only, track its contents as scalars,
     * unless it is an insn array (see the special case below)
     */
    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) {
        int map_off = off + reg->var_off.value;
        u64 val = 0;

        err = bpf_map_direct_read(map, map_off, size, &val, is_ldsx);
        if (err)
            return err;
    ...
}

Since percpu_array_map_ops now sets map_direct_value_addr, a frozen percpu
array created with BPF_F_RDONLY_PROG satisfies this condition (it is rdonly,
has map_direct_value_addr, and is not an insn array).  It enters the branch,
calls bpf_map_direct_read(), gets the new -EINVAL, and returns it.

Does that mean a plain LDX from such a read-only percpu value is now rejected
at load time with a bare -EINVAL and no verbose() message, rather than being
tracked as an unknown scalar the way the const_fold.c path handles it (and the
way insn arrays are explicitly handled in the else-if below)?

Should this branch also exclude BPF_MAP_TYPE_PERCPU_ARRAY, so the load falls
through to mark_reg_unknown() instead of failing?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27147120330