This is an archive of the discontinued LLVM Phabricator instance.

Implement AllocateRWX and ReleaseRWX for NetBSD
ClosedPublic

Authored by krytarowski on Jun 3 2017, 4:31 PM.

Details

Summary

NetBSD ships with PaX MPROTECT disallowing RWX mappings.
There is a solution to bypass this restriction with double mapping
RX (code) and RW (data) using mremap(2) MAP_REMAPDUP.
The initial mapping must be mmap(2)ed with protection:
PROT_MPROTECT(PROT_EXEC).

This functionality to bypass PaX MPROTECT appeared in NetBSD-7.99.72.

This patch fixes 20 failing tests:

  • LLVM :: DebugInfo/debuglineinfo-macho.test
  • LLVM :: DebugInfo/debuglineinfo.test
  • LLVM :: ExecutionEngine/RuntimeDyld/Mips/ELF_Mips64r2N64_PIC_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/Mips/ELF_N32_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/Mips/ELF_N64R6_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/Mips/ELF_O32R6_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/Mips/ELF_O32_PIC_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/COFF_i386.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF-relaxed.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_STT_FILE.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_x64-64_PC8_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_x64-64_PIC_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_PIC-small-relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/ELF_x86_64_StubBuf.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/MachO_empty_ehframe.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/MachO_i386_DynNoPIC_relocations.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/MachO_i386_eh_frame.s
  • LLVM :: ExecutionEngine/RuntimeDyld/X86/MachO_x86-64_PIC_relocations.s

Sponsored by <The NetBSD Foundation>

Diff Detail

Repository
rL LLVM

Event Timeline

krytarowski created this revision.Jun 3 2017, 4:31 PM
joerg edited edge metadata.Jun 6 2017, 2:39 PM

Given that W^X is becoming a lot more popular across systems including SELinux and other variants, I think it would be better to extend MemoryBlock to store separate pointers for W and X mappings. That avoids the complexity of storing the pointer directly in the allocation.

Otherwise, please check for PROT_MPROTECT (and maybe additionally NetBSD) directly, not for the version. The version check is included in libffi only due to some complexity around mmap redefinitions which are not relevant here.

Given that W^X is becoming a lot more popular across systems including SELinux and other variants, I think it would be better to extend MemoryBlock to store separate pointers for W and X mappings. That avoids the complexity of storing the pointer directly in the allocation.

I was thinking about this and I was evaluating this option. My concern whether it is appropriate to include additional internal pointer for all targets unconditionally or just one extra for NetBSD (and other who can catch up).

I will add this pointer without ifdefs.

Otherwise, please check for PROT_MPROTECT (and maybe additionally NetBSD) directly, not for the version. The version check is included in libffi only due to some complexity around mmap redefinitions which are not relevant here.

Sounds good.

Should I make these pointers (code and data) be available to every target or just for NetBSD?

joerg added a comment.Jun 7 2017, 5:46 AM

Given that W^X is becoming a lot more popular across systems including SELinux and other variants, I think it would be better to extend MemoryBlock to store separate pointers for W and X mappings. That avoids the complexity of storing the pointer directly in the allocation.

I was thinking about this and I was evaluating this option. My concern whether it is appropriate to include additional internal pointer for all targets unconditionally or just one extra for NetBSD (and other who can catch up).

I will add this pointer without ifdefs.

There are two possible APIs to consider: add them to MemoryBlock itself or make Memory::AllocateRWX return a pair of MemoryBlocks. With reference counting, the latter would naturally allow dealing with both types, but I don't think we have that currently. This leaves the question whether every MemoryBlock should have this property or whether a new (sub?)class should be created specifically for RW|RX memory.

Add SecondaryAddress in class Memory.
Use "defined(NetBSD) && defined(PROT_MPROTECT)" preprocessor switch.
No local regressions in execution of tests.

On demand I tested the following patch:

$NetBSD$

--- lib/Support/Unix/Memory.inc.orig	2016-12-16 22:52:53.000000000 +0000
+++ lib/Support/Unix/Memory.inc
@@ -195,9 +195,10 @@ Memory::AllocateRWX(size_t NumBytes, con
 #if defined(__APPLE__) && (defined(__arm__) || defined(__arm64__))
   void *pa = ::mmap(start, PageSize*NumPages, PROT_READ|PROT_EXEC,
                     flags, fd, 0);
-#else
-  void *pa = ::mmap(start, PageSize*NumPages, PROT_READ|PROT_WRITE|PROT_EXEC,
-                    flags, fd, 0);
+#elif defined(__NetBSD__) && defined(PROT_MPROTECT)
+  void *pa =
+      ::mmap(start, PageSize * NumPages,
+             PROT_READ | PROT_WRITE | PROT_MPROTECT(PROT_EXEC), flags, fd, 0);
 #endif
   if (pa == MAP_FAILED) {
     if (NearBlock) //Try again without a near hint
@@ -223,10 +224,35 @@ Memory::AllocateRWX(size_t NumBytes, con
     MakeErrMsg(ErrMsg, "vm_protect RW failed");
     return MemoryBlock();
   }
+#elif defined(__NetBSD__) && defined(PROT_MPROTECT) && 0
+  void *codeseg =
+      ::mremap(pa, PageSize * NumPages, NULL, PageSize * NumPages,
+               MAP_REMAPDUP);
+  if (codeseg == MAP_FAILED) {
+    ::munmap(pa, PageSize * NumPages);
+
+    if (NearBlock) // Try again without a near hint
+      return AllocateRWX(NumBytes, nullptr);
+
+    MakeErrMsg(ErrMsg, "Can't allocate RWX Memory");
+    return MemoryBlock();
+  }
+  if (::mprotect(codeseg, PageSize * NumPages, PROT_READ | PROT_EXEC) == -1) {
+    ::munmap(pa, PageSize * NumPages);
+    ::munmap(codeseg, PageSize * NumPages);
+    if (NearBlock) // Try again without a near hint
+      return AllocateRWX(NumBytes, nullptr);
+
+    MakeErrMsg(ErrMsg, "Can't allocate RWX Memory");
+    return MemoryBlock();
+  }
 #endif
 
   MemoryBlock result;
   result.Address = pa;
+#if defined(__NetBSD__) && defined(PROT_MPROTECT) && 0
+  result.SecondaryAddress = codeseg;
+#endif
   result.Size = NumPages*PageSize;
 
   return result;
@@ -236,6 +262,9 @@ bool Memory::ReleaseRWX(MemoryBlock &M, 
   if (M.Address == nullptr || M.Size == 0) return false;
   if (0 != ::munmap(M.Address, M.Size))
     return MakeErrMsg(ErrMsg, "Can't release RWX Memory");
+  if (M.SecondaryAddress)
+    if (0 != ::munmap(M.SecondaryAddress, M.Size))
+      return MakeErrMsg(ErrMsg, "Can't release RWX Memory");
   return false;
 }

There are no regressions compared to the pending patch. I don't understand why it works..

krytarowski edited the summary of this revision. (Show Details)

Upload new revision suggested by Joerg.

joerg accepted this revision.Jun 18 2017, 9:49 AM
This revision is now accepted and ready to land.Jun 18 2017, 9:49 AM
krytarowski closed this revision.Jun 18 2017, 9:53 AM