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

[sashiko-bot@kernel.org]

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.

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;
			}
		}
}

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