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

[sashiko-bot@kernel.org]

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.

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?

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917055645.3926444-1-yonghong.song@linux.dev?part=16