[PATCH] efi: arm,arm64: Fix relocations from not being loaded

[Patrick Zacharias]

I created a GitHub repository demonstrating the issue, together with 
instructions on how to build it.

https://github.com/Fighter19/hello-world-arm-efi

Currently, when using the old linker file, it will crash.
Personally, to test it, I built U-Boot for QEMU and loaded the EFI file 
via TFTP.

Am 06.11.22 um 18:03 schrieb Patrick Zacharias:
> Hi Heinrich,
>
> Thanks for the information, I'll make sure to use it for future 
> contributions.
>
> I encountered this issue, while building a custom EFI application 
> using Rust by linking
> the following files:
>
> ./arch/arm/lib/reloc_arm_efi.c
> ./arch/arm/lib/crt0_arm_efi.S
> ./arch/arm/lib/elf_arm_efi.lds
>
> As well as the custom application built as a library, that exports 
> "efi_main".
>
> Thus, the relocations were caused by the "write_fmt" functions that 
> Rust provides.
> I encountered it, when concatenating two strings.
> format!("Test {}", "Test2");
>
> The relocation was not applied and therefor lead to a crash.
>
> From my understanding these relocations are ELF relocations and are 
> therefor applied by the _relocate function (inside the built application)
> and not by the EFI loader. This is why I assumed, that it's fine for 
> these relocations to reside in the data section.
>
> I noticed, that in the _relocate function in 
> ./arch/arm/lib/reloc_arm_efi.c, that the case R_ARM_RELATIVE was never 
> entered,
> even though the generated shared object included them.
> I then looked at the EFI header file (crt0_arm_efi.S) and noticed, 
> that it specifies only up to _edata to be loaded.
>
> I concluded that _edata needs to be extended to include the ELF 
> relocations as well.
>
> After I decided to place it after the relocations, I noticed, that 
> _edata is already specified at the same (new position) in gnu-efi.
> This reaffirmed me to contribute the patch as it currently is.
>
> I assumed, that on x86, that the linker treats the ELF relocations 
> (.rela.*) as if it was part of the data section.
> From my understanding the EFI relocations are in .reloc while the ELF 
> relocations are in .rela.*.
> I might be mistaken, however.
>
> Greetings,
> Patrick
>
> Am 06.11.22 um 10:20 schrieb Heinrich Schuchardt:
>> On 10/31/22 21:01, Patrick Zacharias wrote:
>>> Prior to this commit, the relocations would not get loaded by the efi
>>> loader.
>>>
>>> This lead to none of the relocations being applied.
>>>
>>> Signed-off-by: Fighter19 <1475802+Fighter19 at users.noreply.github.com>
>>
>> Thanks Patrick for your contribution.
>>
>> You can use scripts/get_maintainer.pl to determine to whom a patch
>> should be sent.
>>
>> Where did you actually see relocations?
>> Which code is not position independent?
>>
>>> ---
>>>   arch/arm/lib/elf_aarch64_efi.lds | 2 +-
>>>   arch/arm/lib/elf_arm_efi.lds     | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/lib/elf_aarch64_efi.lds
>>> b/arch/arm/lib/elf_aarch64_efi.lds
>>> index c0604dad46..1982864d17 100644
>>> --- a/arch/arm/lib/elf_aarch64_efi.lds
>>> +++ b/arch/arm/lib/elf_aarch64_efi.lds
>>> @@ -46,12 +46,12 @@ SECTIONS
>>>           *(COMMON)
>>>           . = ALIGN(512);
>>>           _bss_end = .;
>>> -        _edata = .;
>>>       }
>>>       .rela.dyn : { *(.rela.dyn) }
>>>       .rela.plt : { *(.rela.plt) }
>>>       .rela.got : { *(.rela.got) }
>>>       .rela.data : { *(.rela.data) *(.rela.data*) }
>>> +    _edata = .;
>>>       _data_size = . - _etext;
>>>
>>>       . = ALIGN(4096);
>>> diff --git a/arch/arm/lib/elf_arm_efi.lds 
>>> b/arch/arm/lib/elf_arm_efi.lds
>>> index 767ebda635..c1b58a8033 100644
>>> --- a/arch/arm/lib/elf_arm_efi.lds
>>> +++ b/arch/arm/lib/elf_arm_efi.lds
>>> @@ -46,12 +46,12 @@ SECTIONS
>>>           *(COMMON)
>>>           . = ALIGN(512);
>>>           _bss_end = .;
>>> -        _edata = .;
>>>       }
>>>       .rel.dyn : { *(.rel.dyn) }
>>>       .rel.plt : { *(.rel.plt) }
>>>       .rel.got : { *(.rel.got) }
>>>       .rel.data : { *(.rel.data) *(.rel.data*) }
>>> +    _edata = .;
>>
>> Relocations (if they exist) should be in the .reloc section, not in the
>> .data section.
>>
>> If we want to create a .reloc section, we have to change
>> arch/arm/lib/crt0_*_efi.S too. Furthermore the relocation section must
>> be pointed to by field BaseRelocationTable of the Optional Header Data
>> Directories (see PE-COFF specification).
>>
>> Please, consider the other UEFI architectures (x86 and RISC-V) too.
>>
>> Best regards
>>
>> Heinrich
>>
>>>       _data_size = . - _etext;
>>>
>>>       /DISCARD/ : {
>>