Skip to content

[LLD] Extend special OpenBSD support, but scope under ELFOSABI #97122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jun 28, 2024

  • Add support for .openbsd.mutable

    (rebaser's note) adapted from:
    openbsd/src@bd249b5
    New auto-coalescing sections removed

    In the linkers, collect objects in section "openbsd.mutable" and place
    them into a page-aligned region in the bss, with the right markers for
    kernel/ld.so to identify the region and skip making it immutable. While
    here, fix readelf/objdump versions to show all of this. ok miod kettenis

  • Add support for .openbsd.syscalls

    (rebaser's note) adapted from:
    openbsd/src@42a61ac

    Collect .openbsd.syscalls sections into a new PT_OPENBSD_SYSCALLS
    segment. This will be used soon to pin system calls to designated call
    sites.

    ok deraadt@

  • Scope OpenBSD special section handling under that ELFOSABI

    As a preexisting comment in ELF/Writer.cpp says:

    section names shouldn't be significant in ELF in spirit.

    so scoping OSABI-specific magic name hacks to just the OSABI in
    question limits the degree to which we deviate from that "spirit" for
    all other OSABIs.

    OpenBSD in particular is very fast moving, having added a number of
    special sections, etc. in recent years. It is unclear how possible /
    reasonable it is for upstream to implement all these features in any
    event, but scoping like this at least mitigates the fallout for other
    OSABIs systems which wish to be more slow-moving.

Depends on #98158

@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-mc
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: John Ericson (Ericson2314)

Changes

> (rebaser's note) adapted from:
> openbsd/src@bd249b5
>
> In the linkers, collect objects in section "openbsd.mutable" and place them into a page-aligned region in the bss, with the right markers for kernel/ld.so to identify the region and skip making it immutable. While here, fix readelf/objdump versions to show all of this. ok miod kettenis

> (rebaser's note) adapted from:
> openbsd/src@42a61ac
>
> Collect .openbsd.syscalls sections into a new PT_OPENBSD_SYSCALLS segment. This will be used soon to pin system calls to designated call sites.
>
> ok deraadt@

Note that I think it would seem better if all this OpenBSD stuff was conditioned on config->osabi == ELFOSABI_FREEBSD, but the -m flag for ld.llld is quite underpowered compared to --target.


Full diff: https://github.com/llvm/llvm-project/pull/97122.diff

3 Files Affected:

  • (modified) lld/ELF/LinkerScript.cpp (+2-1)
  • (modified) lld/ELF/ScriptParser.cpp (+2)
  • (modified) lld/ELF/Writer.cpp (+11)
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 1ec796a3bdd99..e1f38eccb1518 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -107,7 +107,8 @@ static StringRef getOutputSectionName(const InputSectionBase *s) {
   for (StringRef v :
        {".data.rel.ro", ".data", ".rodata", ".bss.rel.ro", ".bss", ".ldata",
         ".lrodata", ".lbss", ".gcc_except_table", ".init_array", ".fini_array",
-        ".tbss", ".tdata", ".ARM.exidx", ".ARM.extab", ".ctors", ".dtors"})
+        ".tbss", ".tdata", ".ARM.exidx", ".ARM.extab", ".ctors", ".dtors",
+        ".openbsd.randomdata", ".openbsd.mutable"})
     if (isSectionPrefix(v, s->name))
       return v;
 
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index db46263115242..41bd9a95053f7 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -1628,7 +1628,9 @@ unsigned ScriptParser::readPhdrType() {
                      .Case("PT_GNU_EH_FRAME", PT_GNU_EH_FRAME)
                      .Case("PT_GNU_STACK", PT_GNU_STACK)
                      .Case("PT_GNU_RELRO", PT_GNU_RELRO)
+                     .Case("PT_OPENBSD_MUTABLE", PT_OPENBSD_MUTABLE)
                      .Case("PT_OPENBSD_RANDOMIZE", PT_OPENBSD_RANDOMIZE)
+                     .Case("PT_OPENBSD_SYSCALLS", PT_OPENBSD_SYSCALLS)
                      .Case("PT_OPENBSD_WXNEEDED", PT_OPENBSD_WXNEEDED)
                      .Case("PT_OPENBSD_BOOTDATA", PT_OPENBSD_BOOTDATA)
                      .Default(-1);
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 640cb2a445f7d..917ed48bf0ec4 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2245,11 +2245,22 @@ SmallVector<PhdrEntry *, 0> Writer<ELFT>::createPhdrs(Partition &part) {
     addHdr(PT_GNU_EH_FRAME, part.ehFrameHdr->getParent()->getPhdrFlags())
         ->add(part.ehFrameHdr->getParent());
 
+  // PT_OPENBSD_MUTABLE is an OpenBSD-specific feature. That makes
+  // the dynamic linker fill the segment with zero data, like bss, but
+  // it can be treated differently.
+  if (OutputSection *cmd = findSection(".openbsd.mutable", partNo))
+    addHdr(PT_OPENBSD_MUTABLE, cmd->getPhdrFlags())->add(cmd);
+
   // PT_OPENBSD_RANDOMIZE is an OpenBSD-specific feature. That makes
   // the dynamic linker fill the segment with random data.
   if (OutputSection *cmd = findSection(".openbsd.randomdata", partNo))
     addHdr(PT_OPENBSD_RANDOMIZE, cmd->getPhdrFlags())->add(cmd);
 
+  // PT_OPENBSD_SYSCALLS is an OpenBSD-specific feature. That makes
+  // the kernel and dynamic linker register system call sites.
+  if (OutputSection *cmd = findSection(".openbsd.syscalls", partNo))
+    addHdr(PT_OPENBSD_SYSCALLS, cmd->getPhdrFlags())->add(cmd);
+
   if (config->zGnustack != GnuStackKind::None) {
     // PT_GNU_STACK is a special section to tell the loader to make the
     // pages for the stack non-executable. If you really want an executable

@Ericson2314 Ericson2314 changed the title [LLD] Add support for two special openbsd-sections [LLD] Add support for two special OpenBSD sections Jun 28, 2024
@Ericson2314
Copy link
Member Author

I'm happy to report that with change, my cross compiled statically linked hello world ran successfully :)

@tschuett tschuett requested a review from MaskRay June 29, 2024 03:35
Ericson2314 added a commit to obsidiansystems/nixpkgs that referenced this pull request Jun 30, 2024
@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from 05716a8 to cfbef14 Compare June 30, 2024 23:50
Copy link

github-actions bot commented Jun 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from cfbef14 to c6dafea Compare June 30, 2024 23:55
@Ericson2314
Copy link
Member Author

@MaskRay After testing that my cross compiled binary still runs, I:

  • Removed the section-coalescing parts of the first two commits.

  • Added a new commit which:

    • Gated the PT_-assignments under a config->osabi == ELFOSABI_OPENBSD check

    • Modified LLVM to set ELFOSABI_OPENBSD for OpenBSD targets just like it does ELFOSABI_FREEBSD for FreeBSD ones

I am reasonably happy with this. Per the 3rd commits message, without weighing in on the degree to which it is possible/reasonable for upstream to track something as fast-moving as OpenBSD, the config->osabi == ELFOSABI_OPENBSD adds a stronger "firewall" between OpenBSD and everything else.

@Ericson2314 Ericson2314 changed the title [LLD] Add support for two special OpenBSD sections [LLVM][LLD] Add support for two special OpenBSD sections Jul 1, 2024
@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from c6dafea to f5c3bab Compare July 9, 2024 13:34
@Ericson2314
Copy link
Member Author

I rebased atop #98158 so as to separate the LLVM an LLD parts, in case that is easier for review.

@Ericson2314 Ericson2314 changed the title [LLVM][LLD] Add support for two special OpenBSD sections [LLD] Add support for two special OpenBSD sections Jul 9, 2024
@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from f5c3bab to 7f1f90c Compare July 10, 2024 14:19
@llvmbot llvmbot added the mc Machine (object) code label Jul 10, 2024
@Ericson2314 Ericson2314 changed the title [LLD] Add support for two special OpenBSD sections [LLD] Extend special OpenBSD support, but scope under ELFOSABI Jul 10, 2024
@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch 2 times, most recently from 5a29ce9 to 4010410 Compare July 11, 2024 13:16
@Ericson2314
Copy link
Member Author

OK @MaskRay I think this is ready now?

@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from 4010410 to 397eb87 Compare July 11, 2024 21:27
@Ericson2314
Copy link
Member Author

(Rebased because one thing in #98158 needed fixing.)

@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from 397eb87 to ac43c29 Compare July 11, 2024 21:33
@Ericson2314
Copy link
Member Author

I found another such possibly-affected test. I need to make sure it is working --- even if the section rename is not necessary, I figure either keeping it, or making the target openbsd not linux is a good idea.

s == ".fini_array" || s == ".init_array" ||
s == ".openbsd.randomdata" || s == ".preinit_array";

bool abiAgnostic = s == ".data.rel.ro" || s == ".bss.rel.ro" ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine to keep this unchanged. We can add the osabi check if the .openbsd.* list ever gets longer.

Copy link
Member Author

@Ericson2314 Ericson2314 Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaskRay I would sort of like to change it for consistency.

I ended up duplicating the test I had to change so we have a Linux and OpenBSD version, and this makes me feel better about coverage, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And checked locally that both the new tests worked, and the old test failed.)

- Add support for `.openbsd.mutable`

  (rebaser's note) adapted from:
  openbsd/src@bd249b5
  New auto-coalescing sections removed

  In the linkers, collect objects in section "openbsd.mutable" and place
  them into a page-aligned region in the bss, with the right markers for
  kernel/ld.so to identify the region and skip making it immutable. While
  here, fix readelf/objdump versions to show all of this. ok miod kettenis

- Add support for `.openbsd.syscalls`

  (rebaser's note) adapted from:
  openbsd/src@42a61ac

  Collect .openbsd.syscalls sections into a new PT_OPENBSD_SYSCALLS
  segment. This will be used soon to pin system calls to designated call
  sites.

  ok deraadt@

- Scope OpenBSD special section handling under that ELFOSABI

  As a preexisting comment in `ELF/Writer.cpp` says:

  > section names shouldn't be significant in ELF in spirit.

  so scoping OSABI-specific magic name hacks to just the OSABI in
  question limits the degree to which we deviate from that "spirit" for
  all other OSABIs.

  OpenBSD in particular is very fast moving, having added a number of
  special sections, etc. in recent years. It is unclear how possible /
  reasonable it is for upstream to implement all these features in any
  event, but scoping like this at least mitigates the fallout for other
  OSABIs systems which wish to be more slow-moving.
@Ericson2314 Ericson2314 force-pushed the lld-mutable-syscalls branch from ac43c29 to 76deb33 Compare July 12, 2024 13:10
@Ericson2314
Copy link
Member Author

Linux CI is stuck in the queue, Windows has

PASS: lld :: ELF/openbsd-phdr.s (1928 of 2949)
...
PASS: lld :: ELF/relro-openbsd.s (2129 of 2949)
PASS: lld :: ELF/relro.s (2130 of 2949)

and there is no thing host-specific about this, so I think we are good!

@Ericson2314 Ericson2314 merged commit d7fd8b1 into llvm:main Jul 12, 2024
5 of 6 checks passed
@Ericson2314 Ericson2314 deleted the lld-mutable-syscalls branch July 12, 2024 18:34
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…97122)

- Add support for `.openbsd.mutable`

  (rebaser's note) adapted from:

openbsd/src@bd249b5
  New auto-coalescing sections removed

  In the linkers, collect objects in section "openbsd.mutable" and place
  them into a page-aligned region in the bss, with the right markers for
kernel/ld.so to identify the region and skip making it immutable. While
here, fix readelf/objdump versions to show all of this. ok miod kettenis

- Add support for `.openbsd.syscalls`

  (rebaser's note) adapted from:

openbsd/src@42a61ac

  Collect .openbsd.syscalls sections into a new PT_OPENBSD_SYSCALLS
  segment. This will be used soon to pin system calls to designated call
  sites.

  ok deraadt@

- Scope OpenBSD special section handling under that ELFOSABI

  As a preexisting comment in `ELF/Writer.cpp` says:

  > section names shouldn't be significant in ELF in spirit.

  so scoping OSABI-specific magic name hacks to just the OSABI in
  question limits the degree to which we deviate from that "spirit" for
  all other OSABIs.

  OpenBSD in particular is very fast moving, having added a number of
  special sections, etc. in recent years. It is unclear how possible /
  reasonable it is for upstream to implement all these features in any
  event, but scoping like this at least mitigates the fallout for other
  OSABIs systems which wish to be more slow-moving.

Co-authored-by: deraadt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lld:ELF lld mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants