Hi Jerome,
On 2026-10-27T16:02:16, Jerome Forissier <[email protected]> wrote:
> x86: Handle unordered runtime relocations
>
> Runtime relocation entries are not guaranteed to be sorted by target
> address. The existing code skips every entry whose address is below the
> previous one, leaving it unrelocated.
Please can you say which toolchain or linker options give unsorted
entries, and what goes wrong when it happens? My understanding is that
GNU ld with combreloc (the default) sorts the RELATIVE relocations by
offset, so it would help to know where this was seen, e.g. lld, a
newer binutils or -z nocombreloc
>
> Check that each relocation target lies within the image instead. Apply
> the same correction to the 32-bit and 64-bit paths.
>
> Fixes: b50b1633c022 ("x86: Add support for 64-bit relocation")
> Fixes: 62f7970a5a06 ("x86: Add error checking to x86 relocation code")
> Signed-off-by: Jerome Forissier <[email protected]>
>
> arch/x86/lib/relocate.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
> diff --git a/arch/x86/lib/relocate.c b/arch/x86/lib/relocate.c
> @@ -117,7 +115,7 @@ static void do_elf_reloc_fixups32(unsigned int text_base,
> uintptr_t size,
> /* Check that the location of the relocation is in .text */
> if (offset_ptr_rom >= (Elf32_Addr *)(uintptr_t)text_base &&
> - offset_ptr_rom > last_offset) {
> + offset_ptr_rom < (Elf32_Addr *)(text_base + size)) {
One thing I'd like to check.. the ordering check came from commit
62f7970a, which says it is there to catch corrupted relocation data.
On x86 the .bss is an OVERLAY on __rel_dyn_start in both u-boot.lds
and u-boot-64.lds, so a stray BSS write before relocation overwrites
entries. The 'must increase' test was a cheap way to skip that
garbage. With this patch, a corrupted entry that lands inside the
image is applied. In the 32-bit case that means a '+=' on whatever is
there, as long as the value passes the target check.
I'm fine with the change, since the upper bound covers most of this,
but please can you mention it in the commit message, so it is clear
that the bounds check now does this job. The comment above
do_elf_reloc_fixups() points readers at that commit, so the two should
stay consistent.
Regards,
Simon
