Re: [PATCH bpf-next 16/20] libbpf: Carry the exception cleanup table through the light skeleton

[Yonghong Song <yonghong.song@linux.dev>]

On 9/16/26 11:18 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] libbpf: unconditional cleanup_info emission breaks backward compatibility on older kernels
> - [Medium] libbpf: integer overflow in cleanup_info size calculation leads to OOB read/write
> --
>
> commit 076a2794e54df3a939c1a2aebf4467a8a0af5e41
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> libbpf: Carry the exception cleanup table through the light skeleton
>
> This commit adds support for carrying the exception cleanup table through
> the light skeleton during BPF program loads. It expands the bpf_attr union
> to include cleanup_info fields and handles endianness byte-swapping for the
> records.
>
>> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
>> index af3a04f161ac1..e9bb282a557a3 100644
>> --- a/tools/lib/bpf/gen_loader.c
>> +++ b/tools/lib/bpf/gen_loader.c
> [ ... ]
>> @@ -1011,8 +1016,11 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
>>   			       load_attr->line_info_rec_size;
>>   	int core_relo_tot_sz = gen->core_relo_cnt *
>>   			       sizeof(struct bpf_core_relo);
>> +	int cleanup_info_tot_sz = load_attr->cleanup_info_cnt *
>> +				  load_attr->cleanup_info_rec_size;
> [Severity: Medium]
> Is there a risk of integer overflow here when parsing untrusted ELF files?
>
> If load_attr->cleanup_info_cnt is large enough, the multiplication with
> load_attr->cleanup_info_rec_size could overflow the 32-bit signed integer
> cleanup_info_tot_sz, resulting in a truncated heap allocation.

We should be okay here. In add_data, we will see:

static int add_data(struct bpf_gen *gen, const void *data, __u32 size)
{
         __u64 zero = 0;
         __u32 size8;
         void *prev;

         if (size > INT32_MAX) {
                 gen->error = -ERANGE;
                 return 0;
         }
         size8 = roundup(size, 8);
	...
}

So size guarding above prevents integer overflow. add_data() also
applies to func_info_tot_sz, line_info_tot_sz, core_relo_tot_sz and
here cleanup_info_tot_sz.

>
> Later in info_blob_bswap(), the loop executes load_attr->cleanup_info_cnt
> times:
>
> info_blob_bswap() {
>      ...
>      for (i = 0; i < load_attr->cleanup_info_cnt; i++)
>          bpf_cleanup_info_bswap(ci++);
> }
>
> Could this lead to an out-of-bounds memory read and write over the
> truncated buffer, potentially causing memory corruption in userspace
> tooling like bpftool?
>
> [ ... ]
>> @@ -1085,6 +1101,9 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
>>   	/* populate union bpf_attr with a pointer to core_relos */
>>   	emit_rel_store(gen, attr_field(prog_load_attr, core_relos), core_relos);
>>   
>> +	/* populate union bpf_attr with a pointer to cleanup_info */
>> +	emit_rel_store(gen, attr_field(prog_load_attr, cleanup_info), cleanup_info);
> [Severity: High]
> Will this unconditional emission cause light skeletons to fail to load on
> older kernels?

You are right. We cannot unconditional emit. We can only do emit
if cleanup_info_cnt > 0.

>
> By unconditionally writing a non-zero pointer (the offset in the data blob)
> into the tail of the BPF attribute structure, older target kernels that do
> not support the cleanup_info field might reject the BPF_PROG_LOAD syscall.
>
> Because sys_bpf verifies that all fields beyond its known sizeof(union
> bpf_attr) are zero using bpf_check_uarg_tail_zero(), it seems this could
> fail with -E2BIG. Does this break backward compatibility for all light
> skeletons running on older kernels, even those that do not use exceptions?
>