[lld] [lld/mac] Allow -segprot having stricter initprot than maxprot on mac (PR #107269)
Nico Weber via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 5 09:29:03 PDT 2024
https://github.com/nico updated https://github.com/llvm/llvm-project/pull/107269
>From 8cdecedabf27c3e94dd5831b59e465afeeb5f0e2 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis at chromium.org>
Date: Wed, 4 Sep 2024 10:53:58 -0400
Subject: [PATCH 1/2] [lld/mac] Allow -segprot having stricter initprot than
maxprot on mac
...including for catalyst.
The usecase for this is to put certain security-critical variables into a
special segment/section that's mapped as read-only most of the time, and that
temporary gets remapped as writeable when these variables are written to be the
program. This protects against them being written to by heap spraying attacks.
This special section should be mapped as read-only at program start, so using
`-segprot MY_PROTECTED_MEMORY_THINGER rw r`
to mark that segment as rw maxprot and r initprot is exactly what we want.
lld has so far rejected mismatching initprot and maxprot.
ld64 doesn't reject this, but silently writes initprot into both fields (!)
It looks like this might not be fully intentional, see
https://crbug.com/41495919#comment5 and http://crbug.com/41495919#comment8.
In any case, when postprocessing ld64's output to have different values
for initprot and maxprot, the dynamic loader seems to do the right thing
(see also the previous two links).
The same technique also works on Windows, using both link.exe and
lld-link.exe using `/SECTION:myprotsect,R`.
So, since this is useful, allow it when targeting macOS, and make it
do what you'd expect.
See the PR for the program I used to check that this seems to work.
(I only checked on arm64 macOS 14.5 so far; will run this on many
more systems on bots once this is merged and rolled in.)
---
lld/MachO/Driver.cpp | 17 ++++++++++++---
lld/MachO/OutputSegment.cpp | 6 ++++++
lld/test/MachO/segprot.s | 41 +++++++++++++++++++++++++++++++------
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 6a1ff96ed65697..e3ce6f752b2fb6 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1882,9 +1882,20 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
StringRef segName = arg->getValue(0);
uint32_t maxProt = parseProtection(arg->getValue(1));
uint32_t initProt = parseProtection(arg->getValue(2));
- if (maxProt != initProt && config->arch() != AK_i386)
- error("invalid argument '" + arg->getAsString(args) +
- "': max and init must be the same for non-i386 archs");
+
+ bool allowsDifferentInitAndMaxProt =
+ config->platform() == PLATFORM_MACOS ||
+ config->platform() == PLATFORM_MACCATALYST;
+ if (allowsDifferentInitAndMaxProt) {
+ if (initProt > maxProt)
+ error("invalid argument '" + arg->getAsString(args) +
+ "': init must not be more permissive than max");
+ } else {
+ if (maxProt != initProt && config->arch() != AK_i386)
+ error("invalid argument '" + arg->getAsString(args) +
+ "': max and init must be the same for non-macOS non-i386 archs");
+ }
+
if (segName == segment_names::linkEdit)
error("-segprot cannot be used to change __LINKEDIT's protections");
config->segmentProtections.push_back({segName, maxProt, initProt});
diff --git a/lld/MachO/OutputSegment.cpp b/lld/MachO/OutputSegment.cpp
index 3d8a8eb61a9bba..c320af3fb31772 100644
--- a/lld/MachO/OutputSegment.cpp
+++ b/lld/MachO/OutputSegment.cpp
@@ -42,6 +42,12 @@ static uint32_t initProt(StringRef name) {
static uint32_t maxProt(StringRef name) {
assert(config->arch() != AK_i386 &&
"TODO: i386 has different maxProt requirements");
+ auto it = find_if(
+ config->segmentProtections,
+ [&](const SegmentProtection &segprot) { return segprot.name == name; });
+ if (it != config->segmentProtections.end())
+ return it->maxProt;
+
return initProt(name);
}
diff --git a/lld/test/MachO/segprot.s b/lld/test/MachO/segprot.s
index a4e91d13361050..a5ca8342b41aa2 100644
--- a/lld/test/MachO/segprot.s
+++ b/lld/test/MachO/segprot.s
@@ -2,7 +2,10 @@
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
## Make sure the option parser doesn't think --x and -w are flags.
-# RUN: %lld -dylib -o %t %t.o -segprot FOO rwx xwr -segprot BAR --x --x -segprot BAZ -w -w
+# RUN: %lld -dylib -o %t %t.o \
+# RUN: -segprot FOO rwx xwr \
+# RUN: -segprot BAR --x --x \
+# RUN: -segprot BAZ -w -w
# RUN: llvm-readobj --macho-segment %t | FileCheck %s
# CHECK: Name: FOO
@@ -32,12 +35,38 @@
# CHECK-NEXT: maxprot: -w-
# CHECK-NEXT: initprot: -w-
-# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO rwx rw 2>&1 | FileCheck %s --check-prefix=MISMATCH
-# RUN: not %lld -dylib -o /dev/null %t.o -segprot __LINKEDIT rwx rwx 2>&1 | FileCheck %s --check-prefix=NO-LINKEDIT
-# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO uhh wat 2>&1 | FileCheck %s --check-prefix=MISPARSE
-# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO rwx 2>&1 | FileCheck %s --check-prefix=MISSING
+# RUN: %lld -dylib -o %t.different %t.o -segprot FOO rw r
+# RUN: llvm-readobj --macho-segment %t.different \
+# RUN: | FileCheck %s --check-prefix=DIFFERENT
-# MISMATCH: error: invalid argument '-segprot FOO rwx rw': max and init must be the same for non-i386 archs
+# RUN: %no-arg-lld -arch x86_64 -platform_version "mac catalyst" 14.0.0 17.5 \
+# RUN: -dylib -o /dev/null %t.o -segprot FOO rw r
+# RUN: llvm-readobj --macho-segment %t.different \
+# RUN: | FileCheck %s --check-prefix=DIFFERENT
+
+# DIFFERENT: Name: FOO
+# DIFFERENT-NEXT: Size:
+# DIFFERENT-NEXT: vmaddr:
+# DIFFERENT-NEXT: vmsize:
+# DIFFERENT-NEXT: fileoff:
+# DIFFERENT-NEXT: filesize:
+# DIFFERENT-NEXT: maxprot: rw-
+# DIFFERENT-NEXT: initprot: r--
+
+# RUN: not %no-arg-lld -arch x86_64 -platform_version ios-simulator 14.0 15.0 \
+# RUN: -dylib -o /dev/null %t.o -segprot FOO rwx rw 2>&1 \
+# RUN: | FileCheck %s --check-prefix=MISMATCH
+# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO r rw 2>&1 \
+# RUN: | FileCheck %s --check-prefix=INITTOOPERMISSIVE
+# RUN: not %lld -dylib -o /dev/null %t.o -segprot __LINKEDIT rwx rwx 2>&1 \
+# RUN: | FileCheck %s --check-prefix=NO-LINKEDIT
+# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO uhh wat 2>&1 \
+# RUN: | FileCheck %s --check-prefix=MISPARSE
+# RUN: not %lld -dylib -o /dev/null %t.o -segprot FOO rwx 2>&1 \
+# RUN: | FileCheck %s --check-prefix=MISSING
+
+# MISMATCH: error: invalid argument '-segprot FOO rwx rw': max and init must be the same for non-macOS non-i386 archs
+# INITTOOPERMISSIVE: error: invalid argument '-segprot FOO r rw': init must not be more permissive than max
# NO-LINKEDIT: error: -segprot cannot be used to change __LINKEDIT's protections
# MISPARSE: error: unknown -segprot letter 'u' in uhh
# MISPARSE: error: unknown -segprot letter 'a' in wat
>From e6a44a11f7a90c5fc060ae1e3f25d929f41d2097 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis at chromium.org>
Date: Thu, 5 Sep 2024 12:27:41 -0400
Subject: [PATCH 2/2] add comment
---
lld/MachO/Driver.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index e3ce6f752b2fb6..09a539d71dab3b 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1883,6 +1883,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
uint32_t maxProt = parseProtection(arg->getValue(1));
uint32_t initProt = parseProtection(arg->getValue(2));
+ // FIXME: Check if this works on more platforms.
bool allowsDifferentInitAndMaxProt =
config->platform() == PLATFORM_MACOS ||
config->platform() == PLATFORM_MACCATALYST;
More information about the llvm-commits
mailing list