[lld/mac] Allow -segprot having stricter initprot than maxprot on mac #107269

nico · 2024-09-04T16:36:28Z

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

Since loader support for this on iOS is less clear, keep disallowing it there for now.

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

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

llvmbot · 2024-09-04T16:36:59Z

@llvm/pr-subscribers-lld-macho

@llvm/pr-subscribers-lld

Author: Nico Weber (nico)

Changes

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

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

3 Files Affected:

(modified) lld/MachO/Driver.cpp (+14-3)
(modified) lld/MachO/OutputSegment.cpp (+6)
(modified) lld/test/MachO/segprot.s (+35-6)

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

nico · 2024-09-04T16:39:12Z

To test that it works on macOS:

% % cat -n prot.cc
     1	#include <assert.h>
     2	
     3	#include <mach/mach_init.h>
     4	#include <mach/mach_vm.h>
     5	#include <sys/mman.h>
     6	
     7	#define DECLARE_PROTECTED_DATA constinit
     8	#define DEFINE_PROTECTED_DATA \
     9	  constinit __attribute__((section("PROTECTED_MEMORY, protected_memory")))
    10	
    11	extern char __start_protected_memory __asm(
    12	    "section$start$PROTECTED_MEMORY$protected_memory");
    13	extern char __stop_protected_memory __asm(
    14	    "section$end$PROTECTED_MEMORY$protected_memory");
    15	
    16	void CheckMemoryReadOnly(const void* ptr) {
    17	  mach_port_t object_name;
    18	  vm_region_basic_info_64 region_info;
    19	  mach_vm_size_t size = 1;
    20	  mach_msg_type_number_t count = VM_REGION_BASIC_INFO_COUNT_64;
    21	
    22	  kern_return_t kr = mach_vm_region(
    23	      mach_task_self(), reinterpret_cast<mach_vm_address_t*>(&ptr), &size,
    24	      VM_REGION_BASIC_INFO_64, reinterpret_cast<vm_region_info_t>(&region_info),
    25	      &count, &object_name);
    26	  assert(kr == KERN_SUCCESS &&
    27	         region_info.protection == VM_PROT_READ);
    28	}
    29	
    30	void CheckMemoryReadWrite(const void* ptr) {
    31	  mach_port_t object_name;
    32	  vm_region_basic_info_64 region_info;
    33	  mach_vm_size_t size = 1;
    34	  mach_msg_type_number_t count = VM_REGION_BASIC_INFO_COUNT_64;
    35	
    36	  kern_return_t kr = mach_vm_region(
    37	      mach_task_self(), reinterpret_cast<mach_vm_address_t*>(&ptr), &size,
    38	      VM_REGION_BASIC_INFO_64, reinterpret_cast<vm_region_info_t>(&region_info),
    39	      &count, &object_name);
    40	  assert(kr == KERN_SUCCESS &&
    41	         region_info.protection == (VM_PROT_READ | VM_PROT_WRITE));
    42	}
    43	
    44	// -segprot PROTECTED_MEMORY rw r
    45	
    46	DEFINE_PROTECTED_DATA int foo = 0;
    47	
    48	int main() {
    49	  int r;
    50	
    51	  //r = mprotect((void*)&foo, sizeof(foo), PROT_READ);
    52	  //assert(r == 0);
    53	
    54	  CheckMemoryReadOnly(&foo);
    55	
    56	
    57	  r = mprotect((void*)&foo, sizeof(foo), PROT_READ | PROT_WRITE);
    58	  assert(r == 0);
    59	
    60	  CheckMemoryReadWrite(&foo);
    61	
    62	
    63	  r = mprotect((void*)&foo, sizeof(foo), PROT_READ);
    64	  assert(r == 0);
    65	
    66	  CheckMemoryReadOnly(&foo);
    67	}

% out/gn/bin/clang prot.cc -std=c++20 -segprot PROTECTED_MEMORY rw r -isysroot $(xcrun -show-sdk-path) -fuse-ld=lld

% ./a.out

Without lld, this fails to map the section as rw with mprotect:

% out/gn/bin/clang prot.cc -std=c++20 -segprot PROTECTED_MEMORY rw r -isysroot $(xcrun -show-sdk-path)             
% ./a.out                                                                                             
Assertion failed: (r == 0), function main, file prot.cc, line 58.

To check catalyst, following https://github.com/nico/hack/blob/main/notes/catalyst.md:

% out/gn/bin/clang prot.cc -std=c++20 -segprot PROTECTED_MEMORY rw r -isysroot $(xcrun -show-sdk-path) -fuse-ld=lld -target arm64-apple-ios13.1-macabi 
% codesign --sign - a.out
% ./a.out

zmodem

Sounds reasonable to me.

lld/MachO/Driver.cpp

llvmbot added lld lld:MachO labels Sep 4, 2024

zmodem approved these changes Sep 5, 2024

View reviewed changes

lld/MachO/Driver.cpp Show resolved Hide resolved

add comment

e6a44a1

nico merged commit 62e6c1e into llvm:main Sep 5, 2024
4 of 5 checks passed

nico deleted the lld-mac-initprot branch September 5, 2024 16:29

nico mentioned this pull request Sep 20, 2024

[lld/ELF] Add --override-section-flags flag #109454

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lld/mac] Allow -segprot having stricter initprot than maxprot on mac #107269

[lld/mac] Allow -segprot having stricter initprot than maxprot on mac #107269

Uh oh!

nico commented Sep 4, 2024 •

edited

Loading

Uh oh!

llvmbot commented Sep 4, 2024 •

edited

Loading

Uh oh!

nico commented Sep 4, 2024

Uh oh!

zmodem left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[lld/mac] Allow -segprot having stricter initprot than maxprot on mac #107269

[lld/mac] Allow -segprot having stricter initprot than maxprot on mac #107269

Uh oh!

Conversation

nico commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nico commented Sep 4, 2024

Uh oh!

zmodem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nico commented Sep 4, 2024 •

edited

Loading

llvmbot commented Sep 4, 2024 •

edited

Loading