Re: [PATCH bpf-next v4 2/8] bpf: Introduce global percpu data

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

On 2026/4/17 09:30, Leon Hwang wrote:
> On 15/4/26 10:19, Alexei Starovoitov wrote:
>> On Tue, Apr 14, 2026 at 10:19:22PM +0800, Leon Hwang wrote:
>>> On 2026/4/14 22:10, bot+bpf-ci@kernel.org wrote:
> [...]
>>>> In the v3 series, bpf_map_direct_read() itself had a guard
>>>> (map->map_type != BPF_MAP_TYPE_ARRAY), which protected all callers.
>>>> The v4 moved this to caller-side checks but appears to have missed
>>>> const_reg_xfer().
>>>>
>>>>
>>> Correct.
>>>
>>> Will add a guard in bpf_map_direct_read() in the next revision:
>>>
>>> 	if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
>>> 		return -EINVAL;
>>
>> hold on.
>>  			    map->ops->map_direct_value_addr &&
>> -			    map->map_type != BPF_MAP_TYPE_INSN_ARRAY) {
>> +			    map->map_type != BPF_MAP_TYPE_INSN_ARRAY &&
>> +			    map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY) {
>>
>> map_direct_value_addr() is set, but then immediately disallowed ?
>> Where else it's used?
>>
>> Even if value_addr is working, then map_direct_value_meta() looks broken.
>>

IIUC, map_direct_value_meta() is only used for dumping xlated insns. If
no available map_direct_value_addr(), map_direct_value_meta() won't be
called.

> 
> Ah, let me dive deeper.
> 

As for the above changes, let me explain them using diff snippet.

@@ -5808,6 +5808,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_PERCPU_ARRAY)
+               return -EINVAL;
        err = map->ops->map_direct_value_addr(map, &addr, off);
        if (err)
                return err;

It is to guard percpu_array map against const_reg_xfer(). Instead of
updating const_reg_xfer(), better to update bpf_map_direct_read(). WDYT?


@@ -6362,7 +6364,8 @@ static int check_mem_access(struct
bpf_verifier_env *env, int insn_idx, u32 regn
                        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) {
+                           map->map_type != BPF_MAP_TYPE_INSN_ARRAY &&
+                           map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY) {

Since directly reading data from global percpu data is different from
the way of global data, such reading data for global percpu data must
perform the 'this_cpu_off' to read the data on current CPU.

At the moment of load time, the reading cannot read data like the way of
global data, also cannot cause verifier failure.

I'll add a test against bpf_map_is_rdonly()+percpu_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;

				regs[value_regno].type = SCALAR_VALUE;
				__mark_reg_known(&regs[value_regno], val);

Thanks,
Leon