This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Support custom sections between DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END
ClosedPublic

Authored by MaskRay on Apr 28 2022, 10:40 PM.

Details

Summary

We currently hard code RELRO sections. When a custom section is between
DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END, we may report a spurious
error: section: ... is not contiguous with other relro sections. GNU ld
makes such sections RELRO.

glibc recently switched to default --with-default-link=no. This configuration
places __libc_atexit and others between DATA_SEGMENT_ALIGN and
DATA_SEGMENT_RELRO_END. This patch allows such a ld.bfd --verbose
linker script to be fed into lld.

Diff Detail

Event Timeline

MaskRay created this revision.Apr 28 2022, 10:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2022, 10:40 PM
MaskRay requested review of this revision.Apr 28 2022, 10:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2022, 10:40 PM
MaskRay added a comment.EditedApr 29 2022, 12:49 AM

Related:
I think the original is not contiguous with other relro sections error from https://reviews.llvm.org/D40359 is useful.

I remember that ~2 years some folks asked on llvm-dev how to make a custom section RELRO but that discussion did not lead to any action item.
When building glibc this feature rises again (https://sourceware.org/pipermail/libc-alpha/2022-April/138284.html)
(specifying a full SECTIONS command is clumsy and lld doesn't support dumping its built-in rules).
The best I can come up with is

SECTIONS {
         __libc_subfreeres : { *(__libc_subfreeres) }
         __libc_atexit : { *(__libc_atexit) }
         __libc_IO_vtables : { *(__libc_IO_vtables) }
} INSERT BEFORE .data.rel.ro;

But it will trigger the is not contiguous with other relro sections error.
I wonder whether lld should (un)support this.

Looks sensible to me. One quick question, how does orphan placement relate to RELRO. For example if we make RELRO defined as [first relro section start, last relro section end) between DATA_SECTION_ALIGN and DATA_SECTION_RELRO_END, is there any danger of orphan placement putting a non-relro section inside that range? That case isn't safe for RW as it could be written to at runtime.

I suppose the alternative is to add these custom sections to the isRelroSection() although I don't think that is scalable. Another possible source of custom RELRO sections is #pragma clang section relro="foo" and __attribute__((section("foo"))).

Related:
I think the original is not contiguous with other relro sections error from https://reviews.llvm.org/D40359 is useful.

I remember that ~2 years some folks asked on llvm-dev how to make a custom section RELRO but that discussion did not lead to any action item.
When building glibc this feature rises again (https://sourceware.org/pipermail/libc-alpha/2022-April/138284.html)
(specifying a full SECTIONS command is clumsy and lld doesn't support dumping its built-in rules).
The best I can come up with is

SECTIONS {
         __libc_subfreeres : { *(__libc_subfreeres) }
         __libc_atexit : { *(__libc_atexit) }
         __libc_IO_vtables : { *(__libc_IO_vtables) }
} INSERT BEFORE .data.rel.ro;

But it will trigger the is not contiguous with other relro sections error.
I wonder whether lld should (un)support this.

It maybe an area like orphan sections where a policy can be given. Given that many dynamic loaders can only support one RELRO region we've got a number of choices if we see disjoint RELRO. If there are legitimate alternatives to the default then we could support them via a command line option. My thoughts on the policies when there is no linker script with DATA_SEGMENT_RELRO_END:

  • Error: when RELRO isn't contiguous, we don't know enough about the non-relro sections to know if it is safe or secure enough to promote non-relro to relro
  • Promote-RO: In theory making an RO section RELRO is safe, with only a minor drop in security. Error if the section is RW as we can't know if that is safe to make RELRO
  • First: Make the first contiguous sections RELRO and make all following RELRO sections RW. This could result in loss of expected security properties.
  • All: RELRO is from [first RELRO section start, last RELRO section end). This could cause runtime errors if a RW section in the RELRO region is written to. This is the trust me, I know what I'm doing option.

I think promote-ro could be a safe default.

lld/test/ELF/linkerscript/data-segment-relro.test
72

Would be good to add a test case for orphan sections to make sure that we don't add a RW section into the middle of the RELRO sections, as this could be unsafe. I'm happy that the user takes responsibility for everything they've put in the linker script explicitly.

MaskRay updated this revision to Diff 426922.May 3 2022, 11:39 PM
MaskRay marked an inline comment as done.

add orphan section tests

Thanks for the detailed reply. With this patch, users needing custom RELRO sections can specify a full SECTIONS command.

The current Error behavior seems fine.
I believe Promote-RO is technically possible (only addresses of PT_GNU_RELRO are used; the pt_offset is ignored) but may look strange and may confuse binary manipulation tools (llvm-objcopy, objcopy).

All: RELRO is from [first RELRO section start, last RELRO section end). This could cause runtime errors if a RW section in the RELRO region is written to. This is the trust me, I know what I'm doing option.

This may probably be the most convenient approach if the user just wants to specify a linker script fragment (e.g.SECTIONS { ... } INSERT [BEFORE|AFTER] ...;).
But I agree that it is not a safe default.


Given that there are not demanding needs, perhaps not doing anything is fine for now.

peter.smith accepted this revision.May 4 2022, 12:50 AM

LGTM, thanks for the update.

This revision is now accepted and ready to land.May 4 2022, 12:50 AM
MaskRay edited the summary of this revision. (Show Details)May 4 2022, 1:09 AM
MaskRay retitled this revision from [ELF] Support custom sections before DATA_SEGMENT_RELRO_END to [ELF] Support custom sections between DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END.
This revision was landed with ongoing or failed builds.May 4 2022, 1:10 AM
This revision was automatically updated to reflect the committed changes.