[PATCH v1 1/1] aarch64: add permissions constraints to sections in custom linker scripts
Richard Earnshaw (lists)
Richard.Earnshaw@arm.com
Fri May 16 10:23:08 GMT 2025
On 14/05/2025 11:02, Matthieu Longo wrote:
> On 2025-05-12 14:36, Richard Earnshaw (lists) wrote:
>> On 30/04/2025 12:21, Matthieu Longo wrote:
>>> Several custom linker scripts for AArch64 tests were lacking
>>> permisssions constraints on sections (i.e. Read/Write/Execute),
>>> which resulted in the bundling of all the sections in a same
>>> segment with the union of all the required permissions: RWX.
>>> A segment with such lax permissions constitutes a security
>>> hole, so the linker emits the following warning message:
>>> <ELF file> has a LOAD segment with RWX permissions.
>>> This warning message is noisy in the tests, and has no reason
>>> to exist.
>>>
>>> This patch adds to all AArch64 custom linker scripts used for
>>> testing the required permissions to eliminate the creation of
>>> such a segment by the linker.
>>> ---
>>> ld/testsuite/ld-aarch64/aarch64.ld | 23 ++++++++++++-----
>>> .../ld-aarch64/protections/bti-plt.ld | 19 ++++++++++----
>>> ld/testsuite/ld-aarch64/variant_pcs.ld | 25 +++++++++++++------
>>> 3 files changed, 49 insertions(+), 18 deletions(-)
>>
>> I'm going to caveat my whole reply here with 'this is not an area I have much knowledge of'. But here's my 2c worth:
>>
>>
>>>
>>> diff --git a/ld/testsuite/ld-aarch64/aarch64.ld b/ld/testsuite/ld-aarch64/aarch64.ld
>>> index 75ee3b58934..7ea5de1bdd3 100644
>>> --- a/ld/testsuite/ld-aarch64/aarch64.ld
>>> +++ b/ld/testsuite/ld-aarch64/aarch64.ld
>>> @@ -1,19 +1,30 @@
>>> /* Script for ld testsuite */
>>> OUTPUT_ARCH(aarch64)
>>> ENTRY(_start)
>>> +PHDRS
>>> +{
>>> + phdr_phdrs PT_PHDR PHDRS;
>>> + phdr_text PT_LOAD PHDRS FLAGS(5); /* read + execute */
>>> + phdr_rodata PT_LOAD FLAGS(4); /* read */
>>> + phdr_data PT_LOAD FLAGS(6); /* read + write */
>>> + phdr_dynamic PT_DYNAMIC;
>>> +}
>>> SECTIONS
>>> {
>>> + . = 0x8000 - SIZEOF_HEADERS;
>>> /* Read-only sections, merged into text segment: */
>>> - PROVIDE (__executable_start = 0x8000); . = 0x8000;
>>> - .text :
>>> + PROVIDE (__executable_start = 0x8000);
>>> + . = 0x8000;
>>
>> This seems slightly strange, and not what I'd expect to see in a normal image that has loadable headers. I'd expect that the
>> headers would be loaded at 0x8000 and then __executable_start would be moved after the end of the headers; this would match more naturally with the way a normal dynamically loaded image would be laid out since the whole file would be mmapp()'d into the address space and that wouldn't work with the above (unless the first page were made entirely empty apart from the headers.
>>
>
> I agree with your statement.
> Now, the reason why I did this is to avoid changing the offset in a lot of tests. Since we are talking about tests, and not real use cases, it looked ok to use this workaround to preserve the offsets as much as possible.
> What do you think ? Is it worth fixing the offsets ? Or can we live with this workaround ?
If we aren't trying to execute images based from this script, then it's OK to keep it that way. Perhaps we should add a comment, though just to flag up that this isn't normal (so that people will be more wary of using it as a template for a real-world use-case.
>
>>> + .text :
>>> {
>>> *(.before)
>>> *(.text)
>>> *(.after)
>>> - } =0
>>> + } :phdr_text = 0
>>> . = 0x9000;
>>> - .got : { *(.got) *(.got.plt)}
>>> + .got : { *(.got) *(.got.plt)} :phdr_data
>>> + .dynamic : { *(.dynamic) } :phdr_data :phdr_dynamic
>>> . = 0x12340000;
>>> - .far : { *(.far) }
>>> - .ARM.attributes 0 : { *(.ARM.atttributes) }
>>> + .far : { *(.far) } :phdr_text
>>> + .ARM.attributes 0 : { *(.ARM.atttributes) } :phdr_rodata
>>
>> (there's a typo here that's long-standing (three 't's in .atttributes).
>
> You have good eyes. I didn't spot it at all.
It was underlined in red by my spell-checker. The one earlier on the line wasn't.
>
>> I wouldn't expect the .ARM.attributes section to be loaded at all. They're metadata, like dwarf line tables and not really part of the executable image.
>
> I agree with you. They should not be loaded into memory. PT_NOTE seems more appropriate here.
>
>>
>> R.
>>
>>> }
>>> diff --git a/ld/testsuite/ld-aarch64/protections/bti-plt.ld b/ld/testsuite/ld-aarch64/protections/bti-plt.ld
>>> index 8682623d69b..3d231266f24 100644
>>> --- a/ld/testsuite/ld-aarch64/protections/bti-plt.ld
>>> +++ b/ld/testsuite/ld-aarch64/protections/bti-plt.ld
>>> @@ -1,14 +1,23 @@
>>> OUTPUT_ARCH(aarch64)
>>> ENTRY(_start)
>>> +PHDRS
>>> +{
>>> + phdr_phdrs PT_PHDR PHDRS;
>>> + phdr_text PT_LOAD PHDRS FLAGS(5); /* read + execute */
>>> + phdr_rodata PT_LOAD FLAGS(4); /* read */
>>> + phdr_data PT_LOAD FLAGS(6); /* read + write */
>>> + phdr_dynamic PT_DYNAMIC;
>>> +}
>>> SECTIONS
>>> {
>>> + . = 0x10000 - SIZEOF_HEADERS;
>>> . = 0x10000;
>>> - .rela.plt : { *(.rela.plt) *(.rela.iplt) }
>>> + .rela.plt : { *(.rela.plt) *(.rela.iplt) } :phdr_rodata
>>> . = 0x18000;
>>> - .plt : { *(.plt) *(.iplt) }
>>> + .plt : { *(.plt) *(.iplt) } :phdr_text
>>> . = 0x20000;
>>> - .text : { *(.text) }
>>> + .text : { *(.text) } :phdr_text
>>> . = 0x28000;
>>> - .got : { *(.got) *(.got.plt) }
>>> - .ARM.attributes 0 : { *(.ARM.atttributes) }
>>> + .got : { *(.got) *(.got.plt) } :phdr_data
>>> + .ARM.attributes 0 : { *(.ARM.atttributes) } :phdr_rodata
>>> }
>>> diff --git a/ld/testsuite/ld-aarch64/variant_pcs.ld b/ld/testsuite/ld-aarch64/variant_pcs.ld
>>> index a66a9343a77..e04578d216f 100644
>>> --- a/ld/testsuite/ld-aarch64/variant_pcs.ld
>>> +++ b/ld/testsuite/ld-aarch64/variant_pcs.ld
>>> @@ -1,23 +1,34 @@
>>> /* Script for .variant_pcs symbol tests. */
>>> OUTPUT_ARCH(aarch64)
>>> ENTRY(_start)
>>> +PHDRS
>>> +{
>>> + phdr_phdrs PT_PHDR PHDRS;
>>> + phdr_text PT_LOAD PHDRS FLAGS(5); /* read + execute */
>>> + phdr_rodata PT_LOAD FLAGS(4); /* read */
>>> + phdr_data PT_LOAD FLAGS(6); /* read + write */
>>> + phdr_dynamic PT_DYNAMIC;
>>> +}
>>> SECTIONS
>>> {
>>> + . = 0x8000 - SIZEOF_HEADERS;
>>> /* Read-only sections, merged into text segment: */
>>> - PROVIDE (__executable_start = 0x8000); . = 0x8000;
>>> + PROVIDE (__executable_start = 0x8000);
>>> + . = 0x8000;
>>> .text :
>>> {
>>> *(.before)
>>> *(.text)
>>> *(.after)
>>> - } =0
>>> + } :phdr_text =0
>>> + .plt : { *(.plt) } :phdr_text
>>> . = 0x9000;
>>> - .got : { *(.got) *(.got.plt)}
>>> + .got : { *(.got) *(.got.plt)} :phdr_data
>>> . = 0x10000;
>>> - .rela.dyn : { *(.rela.ifunc) }
>>> + .rela.dyn : { *(.rela.ifunc) } :phdr_rodata
>>> . = 0x11000;
>>> - .rela.plt : { *(.rela.plt) *(.rela.iplt) }
>>> + .rela.plt : { *(.rela.plt) *(.rela.iplt) } :phdr_rodata
>>> . = 0x12340000;
>>> - .far : { *(.far) }
>>> - .ARM.attributes 0 : { *(.ARM.atttributes) }
>>> + .far : { *(.far) } :phdr_text
>>> + .ARM.attributes 0 : { *(.ARM.atttributes) } :phdr_rodata
>>> }
>>
>
More information about the Binutils
mailing list