Re: [PATCH bpf-next 17/20] libbpf: Let the static linker carry .bpf_cleanup relocations

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

On 9/16/26 11:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] libbpf: unaligned memory access in linker for 32-bit relocations
> --
>
> commit 408d36d153c1766697801912b5eb3e8087995ef9
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> libbpf: Let the static linker carry .bpf_cleanup relocations
>
> This commit allows the BPF static linker to process 32-bit relocations
> against non-executable sections by handling R_BPF_64_NODYLD32 and
> R_BPF_64_ABS32 relocation types. It updates the 32-bit values directly in
> the raw data of the relocated section.
>
>> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
>> index 78f92c39290af..e5c06023cb5b1 100644
>> --- a/tools/lib/bpf/linker.c
>> +++ b/tools/lib/bpf/linker.c
> [ ... ]
>
>> @@ -2274,6 +2275,22 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>>   						insn->imm += sec->dst_off / sizeof(struct bpf_insn);
>>   					else
>>   						insn->imm += sec->dst_off;
>> +				} else if (sym_type == R_BPF_64_NODYLD32 ||
>> +					   sym_type == R_BPF_64_ABS32) {
>> +					__u32 *val;
>> +
>> +					/* Two spellings of the one thing: LLVM
>> +					 * emits NODYLD32 for a .long against a
>> +					 * section symbol, GNU as emits ABS32
>> +					 * (bpf_reloc_type_lookup() maps
>> +					 * BFD_RELOC_32 to it), and the value
>> +					 * they describe is the same.
>> +					 */
>> +					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;
> [Severity: Medium]
> Can this lead to an unaligned memory access crash?
>
> The static linker processes these 32-bit relocations against non-executable
> sections by casting the relocated offset to a 32-bit integer pointer and
> directly dereferencing it.
>
> If a user supplies a malformed BPF object file with an unaligned relocation
> offset, this could result in undefined behavior and a crash (SIGBUS) on
> host architectures with strict alignment requirements.

This only happen for a malformed BPF object file and we don't care if
the BPF object file is indeed malformed.

>
> Should linker_sanity_check_elf_relos() be updated to ensure the relocation
> offset is a multiple of 4 for non-executable sections, similar to how it
> checks alignment for SHF_EXECINSTR sections?
>
> tools/lib/bpf/linker.c:linker_sanity_check_elf_relos() {
> 		if (link_sec->shdr->sh_flags & SHF_EXECINSTR) {
> 			if (relo->r_offset % sizeof(struct bpf_insn) != 0) {
> 				pr_warn("ELF relo #%d in section #%zu points to missing symbol #%zu in %s\n",
> 					i, sec->sec_idx, sym_idx, obj->filename);
> 				return -EINVAL;
> 			}
> 		}
> }
>