On 2025/2/26 10:19, Hou Tao wrote:
> Hi,
>
[...]
>> @@ -815,6 +850,8 @@ const struct bpf_map_ops percpu_array_map_ops = {
>> .map_get_next_key = array_map_get_next_key,
>> .map_lookup_elem = percpu_array_map_lookup_elem,
>> .map_gen_lookup = percpu_array_map_gen_lookup,
>> + .map_direct_value_addr = percpu_array_map_direct_value_addr,
>> + .map_direct_value_meta = percpu_array_map_direct_value_meta,
>> .map_update_elem = array_map_update_elem,
>> .map_delete_elem = array_map_delete_elem,
>> .map_lookup_percpu_elem = percpu_array_map_lookup_percpu_elem,
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 9971c03adfd5d..5682546b1193e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -6810,6 +6810,8 @@ static 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_ARRAY)
>> + return -EINVAL;
>
> Is the check still necessary ? Because its caller has already added the
> check of map_type.
Yes. It should check here in order to make sure the code logic in
bpf_map_direct_read() is robust enough.
But in check_mem_access(), if map is a read-only percpu array map, it
should not track its contents as SCALAR_VALUE, because the read-only
.percpu, named .ropercpu, hasn't been supported yet.
Should we implement .ropercpu in this patch set, too?
>> err = map->ops->map_direct_value_addr(map, &addr, off);
>> if (err)
>> return err;
>> @@ -7322,6 +7324,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>> /* if map is read-only, track its contents as scalars */
>> if (tnum_is_const(reg->var_off) &&
>> bpf_map_is_rdonly(map) &&
>> + map->map_type == BPF_MAP_TYPE_ARRAY &&
>> map->ops->map_direct_value_addr) {
>> int map_off = off + reg->var_off.value;
>> u64 val = 0;
>
> Do we also need to check in check_ld_imm() to ensure the dst_reg of
> bpf_ld_imm64 on a per-cpu array will not be treated as a map-value-ptr ?
No. The dst_reg of ld_imm64 for percpu array map must be treated as
map-value-ptr, just like the one for array map.
Global percpu variable is very similar to global variable.
