Hi,

On 1/6/26 1:53 PM, Sascha Hauer wrote:
> Add support for applying dynamic relocations to ELF binaries. This allows
> loading ET_DYN (position-independent) binaries and ET_EXEC binaries at
> custom load addresses.
> 
> Key changes:
> - Add elf_image.reloc_offset to track offset between vaddr and load address
> - Implement elf_compute_load_offset() to calculate relocation offset
> - Add elf_set_load_address() API to specify custom load address
> - Implement elf_find_dynamic_segment() to locate PT_DYNAMIC
> - Add elf_relocate() to apply relocations
> - Provide weak default elf_apply_relocations() stub for unsupported 
> architectures
> - Add ELF dynamic section accessors
> 
> The relocation offset type is unsigned long to properly handle pointer
> arithmetic and avoid casting issues.
> 
> Architecture-specific implementations should override the weak
> elf_apply_relocations() function to handle their relocation types.
> 
> Signed-off-by: Sascha Hauer <[email protected]>
> Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Wrong way round ^

Reviewed-by: Ahmad Fatoum <[email protected]>

But some nitpicks and a question below.

> +static void *elf_get_dest(struct elf_image *elf, void *phdr)

How about elf_phdr_relocated_paddr as a more descriptive name?

> +     if (elf->load_address) {
> +             elf->base_load_addr = elf->load_address;
> +     } else if (elf->type == ET_EXEC) {
> +             elf->base_load_addr = NULL;
> +     } else {
> +             elf->base_load_addr = (void *)(phys_addr_t)min_paddr;
> +     }

Curly braces can be removed.

> +     if (elf->type == ET_EXEC && !elf->load_address) {
> +             elf->reloc_offset = 0;
> +     } else {
> +             elf->reloc_offset = ((unsigned long)elf->base_load_addr - 
> min_vaddr);
> +     }

Ditto.


> +void elf_set_load_address(struct elf_image *elf, void *addr)
> +{
> +     elf->load_address = addr;
> +}
> +
> +static void *elf_find_dynamic_segment(struct elf_image *elf)
> +{
> +     void *buf = elf->hdr_buf;
> +     void *phdr = buf + elf_hdr_e_phoff(elf, buf);
> +     int i;
> +
> +     for (i = 0; i < elf_hdr_e_phnum(elf, buf); i++) {
> +             if (elf_phdr_p_type(elf, phdr) == PT_DYNAMIC) {
> +                     u64 offset = elf_phdr_p_offset(elf, phdr);
> +
> +                     /* If loaded from file, PT_DYNAMIC might not be in 
> hdr_buf */
> +                     if (elf->filename)
> +                             return elf_get_dest(elf, phdr);
> +                     else
> +                             /* Binary in memory, use offset */
> +                             return elf->hdr_buf + offset;

Does elf_get_dest(elf, phdr) not compute the same address that we would
get here?

> +     /* Check that we found exactly one relocation type */
> +     if (rel && rela) {
> +             pr_err("ELF has both REL and RELA relocations\n");
> +             return -EINVAL;
> +     }

Future work could be checking these things at compile time and skipping
the checks in PBL.

> diff --git a/lib/Makefile b/lib/Makefile
> index 
> 6d259dd94e163336d7fdab38c7b74b301aabc5c5..da2c8ffe1dbf512a901295c89494e0837f31a0d9
>  100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -24,6 +24,7 @@ obj-y                       += readkey.o
>  obj-y                        += kfifo.o
>  obj-y                        += libbb.o
>  obj-y                        += libgen.o
> +obj-y                        += elf_reloc.o

Why is this not obj-pbl-y?

Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |


Reply via email to