[PATCH] D124656: [ELF] Support custom sections before DATA_SEGMENT_RELRO_END

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

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")))`.

In D124656#3481926 <https://reviews.llvm.org/D124656#3481926>, @MaskRay wrote:

> 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.



================
Comment at: lld/test/ELF/linkerscript/data-segment-relro.test:72
+  ## The custom section __libc_atexit is made relro.
+  __libc_atexit : { *(__libc_atexit) }
   .got : { *(.got) }
----------------
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. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124656/new/

https://reviews.llvm.org/D124656