Re: [PATCH bpf-next v9 5/9] bpftool: Generate skeleton for global percpu data

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

On 14/7/26 00:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] In light skeletons, `BPF_MAP_TYPE_ARENA` maps with initial values are permanently leaked because `__destroy()` skips them, failing to unmap their memory.
> --
> 
> --- Patch [5]: [PATCH bpf-next v9 5/9] bpftool: Generate skeleton for global percpu data ---
> commit b822b756947c191db53a46cfd85674008d72587c
> Author: Leon Hwang <leon.hwang@linux.dev>
> 
> bpftool: Generate skeleton for global percpu data
> 
> Enhance bpftool to generate skeletons that properly handle global percpu
> variables. The generated skeleton now includes a dedicated structure for
> percpu data, allowing users to initialize and access percpu variables more
> efficiently.
> 
>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
>> index a01d06d22d1a3..b60db11081497 100644
>> --- a/tools/bpf/bpftool/gen.c
>> +++ b/tools/bpf/bpftool/gen.c
> 
> [ ... ]
> 
>> @@ -669,7 +693,8 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
>>  		if (!get_map_ident(map, ident, sizeof(ident)))
>>  			continue;
>>  		if (bpf_map__is_internal(map) &&
>> -		    (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
>> +		    ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
>> +		     bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY))
>>  			printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zu);\n",
>>  			       ident, bpf_map_mmap_sz(map));
> 
> [Severity: High]
> This is a pre-existing issue, but does this manual internal map check cause
> BPF_MAP_TYPE_ARENA maps to be permanently leaked in light skeletons?
> 


No, this is not a pre-existing issue.

When tried to generate light skeleton for internal arena map, got:

  GEN-SKEL     [test_progs] arena.lskel.h
Bus error

Instead of such implicit error, I'd like to send a separate patch to
reject generating light skeleton for internal arena map explicitly.

Thanks,
Leon

> ARENA maps with initial values appear to be skipped here because
> bpf_map__is_internal() evaluates to false for them. This skips the generation
> of the skel_free_map_data() call.
> 
> Since ARENA maps can be very large, skipping them during skeleton destruction
> leaks both the userspace virtual memory (VMA) and the underlying kernel
> bpf_map object, as the VMA holds a file reference to it. This can lead to
> resource exhaustion if a daemon repeatedly reloads BPF policies.
> 
> Could this code be updated to use is_skel_data() instead of get_map_ident()
> and bpf_map__is_internal() so that ARENA map memory is correctly unmapped and
> freed?
> 
>>  		codegen("\
>