Re: [PATCH bpf-next v7 11/11] selftests/bpf: Add test to verify bpf_iter for global percpu data

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

On 22/6/26 23:08, sashiko-bot@kernel.org wrote:
[...]
>> diff --git a/tools/testing/selftests/bpf/prog_tests/global_data_init.c b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
>> index e4c290492ff8a..32734e1dbfb00 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/global_data_init.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> [ ... ]
>> @@ -279,6 +279,56 @@ static void test_global_percpu_data_verifier_log(void)
>>  	RUN_TESTS(test_global_percpu_data);
>>  }
>>  
>> +static void test_global_percpu_data_iter(void)
>> +{
>> +	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
>> +	struct test_global_percpu_data *skel;
>> +	union bpf_iter_link_info linfo = {};
>> +	struct bpf_link *link = NULL;
>> +	int fd, num_cpus, len, err;
>> +	char buf[16];
>> +
>> +	num_cpus = libbpf_num_possible_cpus();
>> +	if (!ASSERT_GT(num_cpus, 0, "libbpf_num_possible_cpus"))
>> +		return;
>> +
>> +	skel = test_global_percpu_data__open();
>> +	if (!ASSERT_OK_PTR(skel, "test_global_percpu_data__open"))
>> +		return;
>> +
>> +	skel->rodata->num_cpus = num_cpus;
>> +	skel->rodata->offsetof_num = offsetof(struct test_global_percpu_data__percpu, struct_data);
>> +	skel->rodata->offsetof_num += sizeof(skel->percpu->struct_data) - sizeof(int);
>> +	skel->rodata->elem_sz = roundup(sizeof(struct test_global_percpu_data__percpu), 8);
> 
> [Severity: High]
> This is a pre-existing issue, but does iterating over per-CPU maps expose
> uninitialized kernel heap memory?
> 
> When working with per-CPU maps, temporary buffers are allocated using kmalloc
> without the __GFP_ZERO flag in functions like bpf_iter_init_array_map in
> kernel/bpf/arraymap.c:
> 
> kernel/bpf/arraymap.c:bpf_iter_init_array_map() {
>     ...
>     value_buf = kmalloc(buf_size, GFP_USER | __GFP_NOWARN);
>     ...
> }
> 
> This is also done in kernel/bpf/hashtab.c:bpf_iter_init_hash_map().
> 
> If the map contains a BTF record, bpf_obj_memcpy in include/linux/bpf.h
> explicitly stops at map->value_size instead of filling the entire rounded-up
> size:
> 
> include/linux/bpf.h:bpf_obj_memcpy() {
>     ...
>     memcpy(dst + curr_off, src + curr_off, size - curr_off);
>     ...
> }
> 
> This fails to overwrite the padding bytes up to round_up(map->value_size, 8).
> Because the BPF verifier allows the iterator program to read the entire
> round_up(map->value_size, 8) * num_possible_cpus() bytes, the uninitialized
> padding from kmalloc is exposed to the BPF program.
> 
> Similarly, in kernel/bpf/syscall.c:map_lookup_elem(), kvmalloc is used
> without zeroing:
> 
> kernel/bpf/syscall.c:map_lookup_elem() {
>     ...
>     value = kvmalloc(value_size, GFP_USER | __GFP_NOWARN);
>     ...
> }
> 
> This means sys_bpf BPF_MAP_LOOKUP_ELEM also copies this uninitialized padding
> back to user space via copy_to_user, causing a direct kernel heap info leak.
> 
> Could this be addressed by zeroing these allocations or ensuring the padding is
> cleared when copying map values?
> 

Will verify the issue by separate selftests.

If true, will post separate patch to fix it.

Thanks,
Leon

>> +	skel->percpu->struct_data.nums[6] = 0xc0de;
>> +
>> +	err = test_global_percpu_data__load(skel);
> [ ... ]
>