Re: [PATCH bpf-next v9 19/23] libbpf: Let the static linker carry .bpf_cleanup relocations

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

On 10/8/26 10:14 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] libbpf: OOB array access in linker_append_elf_relos via malformed ELF symbol
> --
>
> commit c7a7a96d1dd021ce6e4bcaa13fd9ec618fb1f8cf
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> libbpf: Let the static linker carry .bpf_cleanup relocations
>
> Each field of a .bpf_cleanup record is a byte offset into a code section: a
> 32-bit relocation against that section's symbol, with the offset as the
> implicit addend. The patch updates the linker to handle R_BPF_64_NODYLD32
> and R_BPF_64_ABS32 relocations against non-executable sections.
>
>> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
>> index f3f71c452f007..c607f51e3cb46 100644
>> --- a/tools/lib/bpf/linker.c
>> +++ b/tools/lib/bpf/linker.c
> [ ... ]
>
>> @@ -2297,6 +2299,39 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>>   					if (linker->swapped_endian)
>>   						off = bswap_64(off);
>>   					memcpy(ptr, &off, sizeof(off));
>> +				} else if ((sym_type == R_BPF_64_NODYLD32 ||
>> +					    sym_type == R_BPF_64_ABS32) &&
>> +					   (sec->shdr->sh_flags & SHF_EXECINSTR)) {
> [Severity: Medium]
> Could this result in an out-of-bounds memory access?
>
> If an ELF file contains a malformed STT_SECTION symbol with an st_shndx

We should not care this since the elf file is *malformed*.

> value >= SHN_LORESERVE (such as SHN_ABS), it bypasses the bounds check against
> obj->sec_cnt in linker_sanity_check_elf_symtab():
>
>      if (sym->st_shndx < SHN_LORESERVE && sym->st_shndx >= obj->sec_cnt) { ... }
>
> When processing relocations against this symbol, linker_append_elf_relos()
> indexes the obj->secs array using this out-of-bounds st_shndx:
>
>      struct src_sec *sec = &obj->secs[src_sym->st_shndx];
>
> The newly added relocation handling branch for R_BPF_64_NODYLD32 and
> R_BPF_64_ABS32 then directly dereferences the out-of-bounds sec->shdr and
> sec->dst_off pointers. Can this lead to an out-of-bounds read and a
> segmentation fault when linking an untrusted ELF file?
>
>> +					/*
>> +					 * A byte offset into a code section,
>> +					 * stored in place. LLVM spells this
>> +					 * relocation NODYLD32 and GNU as
>> +					 * spells it ABS32; being bytes, the
>> +					 * section's new start goes in as it
>> +					 * is, not scaled the way a call's
>> +					 * instruction index is above.
>> +					 *
>> +					 * r_offset is checked only for an
>> +					 * executable section, and SHT_NOBITS
>> +					 * has no raw_data, so bound it here --
>> +					 * subtracting, so it cannot wrap.
>> +					 */
>> +					if (!dst_linked_sec->raw_data ||
>> +					    dst_linked_sec->sec_sz < (int)sizeof(*val) ||
>> +					    dst_rel->r_offset % sizeof(*val) ||
>> +					    dst_rel->r_offset >
>> +					    (size_t)dst_linked_sec->sec_sz - sizeof(*val)) {
>> +						pr_warn("ELF relo #%d in section #%zu points outside the data of section '%s' in %s\n",
>> +							j, src_sec->sec_idx,
>> +							dst_linked_sec->sec_name,
>> +							obj->filename);
>> +						return -EINVAL;
>> +					}
>> +					val = dst_linked_sec->raw_data + dst_rel->r_offset;
>> +					if (linker->swapped_endian)
>> +						*val = bswap_32(bswap_32(*val) + sec->dst_off);
>> +					else
>> +						*val += sec->dst_off;
>>   				} else {
>>   					pr_warn("relocation against STT_SECTION in non-exec section is not supported!\n");
>>   					return -EINVAL;