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 23/6/26 12:05, Leon Hwang wrote:
> 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).


True.

The padding bytes of non-zeroed k/kvmalloc() could be exposed to user space.

Verified the issue by a LOOKUP_ELEM test:
https://github.com/Asphaltt/bpf/commits/bpf/avoid-uninit-mem/v1/.

test_map_uninit_mem_exposure:FAIL:zeroed tail bytes unexpected memory
mismatch
actual:
        2B 2B 2B 2B
expected:
        00 00 00 00

Will post the fix later.

Thanks,
Leon

>> 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);
>> [ ... ]
>>
>