* [PATCH] efi: arm,arm64: Fix relocations from not being loaded
@ 2022-10-31 20:01 Patrick Zacharias
2022-11-06 9:20 ` Heinrich Schuchardt
0 siblings, 1 reply; 7+ messages in thread
From: Patrick Zacharias @ 2022-10-31 20:01 UTC (permalink / raw)
To: u-boot
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@users.noreply.github.com>
---
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 = .;
_data_size = . - _etext;
/DISCARD/ : {
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-10-31 20:01 [PATCH] efi: arm,arm64: Fix relocations from not being loaded Patrick Zacharias
@ 2022-11-06 9:20 ` Heinrich Schuchardt
2022-11-06 17:03 ` Patrick Zacharias
0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2022-11-06 9:20 UTC (permalink / raw)
To: Patrick Zacharias; +Cc: u-boot, Ilias Apalodimas
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@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/ : {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-11-06 9:20 ` Heinrich Schuchardt
@ 2022-11-06 17:03 ` Patrick Zacharias
2022-11-06 18:36 ` Patrick Zacharias
2022-11-06 21:37 ` Heinrich Schuchardt
0 siblings, 2 replies; 7+ messages in thread
From: Patrick Zacharias @ 2022-11-06 17:03 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot, Ilias Apalodimas
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@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/ : {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-11-06 17:03 ` Patrick Zacharias
@ 2022-11-06 18:36 ` Patrick Zacharias
2022-11-06 22:02 ` Heinrich Schuchardt
2022-11-06 21:37 ` Heinrich Schuchardt
1 sibling, 1 reply; 7+ messages in thread
From: Patrick Zacharias @ 2022-11-06 18:36 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot, Ilias Apalodimas
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@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/ : {
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-11-06 17:03 ` Patrick Zacharias
2022-11-06 18:36 ` Patrick Zacharias
@ 2022-11-06 21:37 ` Heinrich Schuchardt
2022-11-07 16:47 ` Patrick Zacharias
1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2022-11-06 21:37 UTC (permalink / raw)
To: Patrick Zacharias; +Cc: u-boot, Ilias Apalodimas
Am 6. November 2022 18:03:09 MEZ schrieb Patrick Zacharias <littlefighter19@web.de>:
>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
The U-Boot files are only used for building test applications for U-Boot. They are not usable for building generic applications which may require relocations.
If your application have different requirements, please create files in your own repository and adjust them to your needs.
When building rust UEFI applications consider using the aarch64-unknown-uefi target and the r_efi crate. That target has been promoted to tier 2 recently. See https://github.com/rust-lang/compiler-team/issues/555
Probably you need a nightly build of the compiler to use it.
Best regards
Heinrich
>
>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@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/ : {
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-11-06 18:36 ` Patrick Zacharias
@ 2022-11-06 22:02 ` Heinrich Schuchardt
0 siblings, 0 replies; 7+ messages in thread
From: Heinrich Schuchardt @ 2022-11-06 22:02 UTC (permalink / raw)
To: Patrick Zacharias; +Cc: u-boot, Ilias Apalodimas
Am 6. November 2022 19:36:15 MEZ schrieb Patrick Zacharias <littlefighter19@web.de>:
>I created a GitHub repository demonstrating the issue, together with instructions on how to build it.
>
>https://github.com/Fighter19/hello-world-arm-efi
Please, check the instructions in
https://doc.rust-lang.org/rustc/platform-support/unknown-uefi.html
Best regards
Heinrich
>
>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@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/ : {
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi: arm,arm64: Fix relocations from not being loaded
2022-11-06 21:37 ` Heinrich Schuchardt
@ 2022-11-07 16:47 ` Patrick Zacharias
0 siblings, 0 replies; 7+ messages in thread
From: Patrick Zacharias @ 2022-11-07 16:47 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot, Ilias Apalodimas
Hi Heinrich,
Thanks for the suggestions.
I will continue investigating, if there is another way to build PE32+
ARM (v6kz) binaries.
I am not aware of any currently working configuration other than the one
through GNU-EFI (aarch64 is not an option for me).
Nonetheless, this patch isn't about me trying to run an EFI binary, it's
about U-Boot.
U-Boot uses GNU-EFI files for building the test applications. And does
so by using bad linker scripts, that weren't present in that form in
GNU-EFI to begin with.
This means that _relocate is fundamentally broken without this patch on
U-Boot.
If this patch isn't applied, the checks in Makefile (looking at the
target checkarmreloc) create a false sense,
that such binaries (that contain relocations) would work.
It's true, that none of the generated binaries contain relocations and
thereby no issues are visible currently,
nonetheless, once they would be introduced, the behavior would cause
confusion.
Greetings,
Patrick
Am 06.11.22 um 22:37 schrieb Heinrich Schuchardt:
> Am 6. November 2022 18:03:09 MEZ schrieb Patrick Zacharias <littlefighter19@web.de>:
>> 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
> The U-Boot files are only used for building test applications for U-Boot. They are not usable for building generic applications which may require relocations.
>
> If your application have different requirements, please create files in your own repository and adjust them to your needs.
>
> When building rust UEFI applications consider using the aarch64-unknown-uefi target and the r_efi crate. That target has been promoted to tier 2 recently. See https://github.com/rust-lang/compiler-team/issues/555
>
> Probably you need a nightly build of the compiler to use it.
>
> Best regards
>
> Heinrich
>
>> 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@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/ : {
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-07 16:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-31 20:01 [PATCH] efi: arm,arm64: Fix relocations from not being loaded Patrick Zacharias
2022-11-06 9:20 ` Heinrich Schuchardt
2022-11-06 17:03 ` Patrick Zacharias
2022-11-06 18:36 ` Patrick Zacharias
2022-11-06 22:02 ` Heinrich Schuchardt
2022-11-06 21:37 ` Heinrich Schuchardt
2022-11-07 16:47 ` Patrick Zacharias
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.