Re: [PATCH bpf-next v2 4/7] libbpf: Skip initial_value override on signed loaders

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer overflow in `add_data()`'s `roundup(size, 8)` can cause a massive out-of-bounds `memcpy()`, leading to a heap buffer overflow.
--

commit aaabe67208a39bc461c4602ad17bdd3b78f93227
Author: Daniel Borkmann <daniel@iogearbox.net>

    libbpf: Skip initial_value override on signed loaders

    bpf_gen__map_update_elem() emits code that, when the host-supplied
    loader ctx provides a non-NULL map_desc[idx].initial_value, overwrites
    the blob value with bytes read from the host (bpf_copy_from_user /
    bpf_probe_read_kernel) before the BPF_MAP_UPDATE_ELEM that populates
    the program's .data/.rodata/.bss maps. This override runs after
    emit_signature_match() has validated map->sha[], which lets an untrusted
    host substitute global-variable contents. This patch skips emitting the
    override for signed loaders.

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index 66a02039da8ce..a5d9c7a5261b0 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c
> @@ -1187,27 +1187,36 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *pvalue,
>  	value = add_data(gen, pvalue, value_size);
>  	key = add_data(gen, &zero, sizeof(zero));

[Severity: High]
This isn't a bug introduced by this patch, but could this call to add_data()
lead to a heap buffer overflow if value_size is extremely large?

Looking at tools/lib/bpf/gen_loader.c:add_data():

static int add_data(struct bpf_gen *gen, const void *data, __u32 size)
{
	__u32 size8 = roundup(size, 8);
...
	if (realloc_data_buf(gen, size8))
		return 0;
	prev = gen->data_cur;
	if (data) {
		memcpy(gen->data_cur, data, size);

If a large size is passed (for example, 0xFFFFFFFC), the roundup(size, 8)
macro would evaluate to size + 7, overflowing the 32-bit unsigned integer
and wrapping around to 0.

This would cause realloc_data_buf() to successfully resize based on the
wrapped size, followed by memcpy() attempting to copy nearly 4GB of data
out of bounds.

Can a maliciously crafted BPF ELF file trigger this integer overflow during
skeleton generation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601150248.394863-1-daniel@iogearbox.net?part=4