From 5ec71d45e829daba110f34d5764ba4de3d64d98d Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Tue, 27 Mar 2018 11:22:05 -0600 Subject: [PATCH] Restrict W+X based on signal handlers to intended pages Make 9d0ab74e9e more responsible by limiting permission changes to pages that are intended to be both writable and executable for code generation. That way, the signal handler doesn't just reopen holes in loaded foreign libraries that W^X would close. --- racket/src/racket/sgc/Makefile.in | 3 +- racket/src/racket/sgc/sgc.c | 17 +++- racket/src/racket/sgc/sgc.h | 3 + racket/src/racket/src/salloc.c | 151 ++++++++++++++++++++++++++---- 4 files changed, 153 insertions(+), 21 deletions(-) diff --git a/racket/src/racket/sgc/Makefile.in b/racket/src/racket/sgc/Makefile.in index 0de1b5a28f..8b1245adbc 100644 --- a/racket/src/racket/sgc/Makefile.in +++ b/racket/src/racket/sgc/Makefile.in @@ -36,7 +36,8 @@ test: $(OBJS) test.@LTO@ gcobjects: $(OBJS) EXTRA_DEPS = $(srcdir)/autostat.inc $(srcdir)/collect.inc \ - $(srcdir)/../utils/splay.c $(srcdir)/../utils/schiptr.h + $(srcdir)/../utils/splay.c $(srcdir)/../utils/schiptr.h \ + $(srcdir)/../sconfig.h sgc.@LTO@: $(srcdir)/sgc.c $(EXTRA_DEPS) $(CC) $(CFLAGS) $(CPPFLAGS) @OPTIONS@ -DSGC_EXPORTS -I.. -c $(srcdir)/sgc.c -o sgc.@LTO@ diff --git a/racket/src/racket/sgc/sgc.c b/racket/src/racket/sgc/sgc.c index 92e0ed3027..5b2abd9179 100644 --- a/racket/src/racket/sgc/sgc.c +++ b/racket/src/racket/sgc/sgc.c @@ -832,6 +832,7 @@ GC_collect_end_callback_Proc GC_set_collect_end_callback(GC_collect_end_callback return old; } +GC_register_as_executable_callback_Proc GC_register_as_executable_callback; static intptr_t roots_count; static intptr_t roots_size; @@ -961,6 +962,7 @@ static void *mmap_sector(int count, int executable) { uintptr_t pre_extra; void *p; + int prot; #ifdef MAP_ANON int fd = -1; int flags = MAP_ANON; @@ -972,14 +974,16 @@ static void *mmap_sector(int count, int executable) fd = open("/dev/zero", O_RDWR); #endif + prot = PROT_READ | PROT_WRITE | (executable ? PROT_EXEC : 0); + #ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER - executable = 0; + if (executable) + prot -= PROT_EXEC; #endif - p = mmap(NULL, (count + 1) << LOG_SECTOR_SEGMENT_SIZE, - PROT_READ | PROT_WRITE | (executable ? PROT_EXEC : 0), + p = mmap(NULL, (count + 1) << LOG_SECTOR_SEGMENT_SIZE, prot, MAP_PRIVATE | flags, fd, 0); - + pre_extra = (uintptr_t)p & (SECTOR_SEGMENT_SIZE - 1); if (pre_extra) pre_extra = SECTOR_SEGMENT_SIZE - pre_extra; @@ -989,12 +993,17 @@ static void *mmap_sector(int count, int executable) munmap((char *)p + pre_extra + (count << LOG_SECTOR_SEGMENT_SIZE), SECTOR_SEGMENT_SIZE - pre_extra); + if (executable && GC_register_as_executable_callback) + GC_register_as_executable_callback((char *)p + pre_extra, count << LOG_SECTOR_SEGMENT_SIZE, 1); + return (char *)p + pre_extra; } static void munmap_sector(void *p, int count) { munmap(p, count << LOG_SECTOR_SEGMENT_SIZE); + if (GC_register_as_executable_callback) + GC_register_as_executable_callback((char *)p, count << LOG_SECTOR_SEGMENT_SIZE, 0); } static void *os_alloc_pages(size_t len) diff --git a/racket/src/racket/sgc/sgc.h b/racket/src/racket/sgc/sgc.h index ba58ceda46..64234bb796 100644 --- a/racket/src/racket/sgc/sgc.h +++ b/racket/src/racket/sgc/sgc.h @@ -54,6 +54,9 @@ typedef void (*GC_collect_end_callback_Proc)(void); SGC_EXTERN GC_collect_start_callback_Proc GC_set_collect_start_callback(GC_collect_start_callback_Proc); SGC_EXTERN GC_collect_end_callback_Proc GC_set_collect_end_callback(GC_collect_end_callback_Proc); +typedef void (*GC_register_as_executable_callback_Proc)(void *p, size_t sz, int can_exec); +SGC_EXTERN GC_register_as_executable_callback_Proc GC_register_as_executable_callback; + SGC_EXTERN void GC_free(void *); /* ... but only if it's turned on in sgc.c. */ struct GC_Set; diff --git a/racket/src/racket/src/salloc.c b/racket/src/racket/src/salloc.c index 54f8ae932c..6b0a181ffd 100644 --- a/racket/src/racket/src/salloc.c +++ b/racket/src/racket/src/salloc.c @@ -111,6 +111,7 @@ static void init_allocation_callback(void); #ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER static void install_w_xor_x_handler(); +static void register_as_executable(void *p, size_t len, int can_exec); #endif SHARED_OK static int use_registered_statics; @@ -162,6 +163,9 @@ void scheme_set_stack_base(void *base, int no_auto_statics) XFORM_SKIP_PROC #ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER install_w_xor_x_handler(); +# if defined(USE_SENORA_GC) && !defined(MZ_PRECISE_GC) + GC_register_as_executable_callback = register_as_executable; +# endif #endif } @@ -1039,6 +1043,10 @@ static void *malloc_page(intptr_t size) if (!r) scheme_raise_out_of_memory(NULL, NULL); +#ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER + register_as_executable(r, size, 1); +#endif + return r; } @@ -1051,6 +1059,9 @@ static void free_page(void *p, intptr_t size) VirtualFree(p, 0, MEM_RELEASE); #else munmap(p, size); +# ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER + register_as_executable(p, size, 0); +# endif #endif } @@ -1433,21 +1444,116 @@ void scheme_notify_code_gc() #ifdef IMPLEMENT_WRITE_XOR_EXECUTE_BY_SIGNAL_HANDLER -/* We abide by W^X by following the letter of the law, but not the - sprirt. Pages are mapped without execute mode. When the process - crashes by trying to execute code from those pages, we switch the - page from writable to executable --- or vice versa if the process - changes back to writing. */ +/* We abide by W^X for generated code by following the letter of the + law, but not the sprirt. Pages are mapped without execute mode. + When the process crashes by trying to execute code from those + pages, we switch the page from writable to executable --- or vice + versa if the process changes back to writing. */ # include # include static intptr_t wx_page_size; -static int try_x = 0; +static int wx_log_page_size; static void (*previous_fault_handler)(int sn, siginfo_t *si, void *ctx); +typedef char exec_state_t; +#define EXEC_STATE_NONE 0 +#define EXEC_STATE_WRITE 1 +#define EXEC_STATE_EXEC 2 +static exec_state_t ***exec_page_map; + +#ifdef MZ_USE_MZRT +static mzrt_mutex *exec_page_mutex = NULL; +#endif + +#ifdef SIXTY_FOUR_BIT_INTEGERS +# define EXEC_PAGEMAP_LEVEL_BITS 22 +#else +# define EXEC_PAGEMAP_LEVEL_BITS 8 +#endif + +static void exec_state_lock() +{ + /* We assume that allocation functions that manipulate the + executable-state table will not themselves trip into + execute--write mismatches that would deadlock via the signal + handler. */ +#ifdef MZ_USE_MZRT + mzrt_mutex_lock(exec_page_mutex); +#endif +} + +static void exec_state_unlock() +{ +#ifdef MZ_USE_MZRT + mzrt_mutex_unlock(exec_page_mutex); +#endif +} + +/* Call with lock: */ +static void exec_pagemap_set(void *p, exec_state_t es) { + uintptr_t addr, pos; + exec_state_t **p1, *p2; + + addr = ((uintptr_t)p) >> wx_log_page_size; + + if (!exec_page_map) + exec_page_map = calloc(sizeof(exec_state_t**), ((sizeof(void*) << 3) - wx_log_page_size - (2 * EXEC_PAGEMAP_LEVEL_BITS))); + + pos = addr >> (2 * EXEC_PAGEMAP_LEVEL_BITS); + p1 = exec_page_map[pos]; + if (!p1) { + p1 = calloc(sizeof(exec_state_t*), (1 << EXEC_PAGEMAP_LEVEL_BITS)); + exec_page_map[pos] = p1; + } + + pos = (addr >> EXEC_PAGEMAP_LEVEL_BITS) & ((1 << EXEC_PAGEMAP_LEVEL_BITS) - 1); + p2 = p1[pos]; + if (!p2) { + p2 = calloc(sizeof(exec_state_t), (1 << EXEC_PAGEMAP_LEVEL_BITS)); + p1[pos] = p2; + } + + pos = addr & ((1 << EXEC_PAGEMAP_LEVEL_BITS) - 1); + p2[pos] = es; +} + +/* Call with lock: */ +static exec_state_t exec_pagemap_get(void *p) { + uintptr_t addr, pos; + exec_state_t **p1, *p2; + + addr = ((uintptr_t)p) >> wx_log_page_size; + + if (!exec_page_map) return EXEC_STATE_NONE; + + pos = addr >> (2 * EXEC_PAGEMAP_LEVEL_BITS); + p1 = exec_page_map[pos]; + if (!p1) return EXEC_STATE_NONE; + + pos = (addr >> EXEC_PAGEMAP_LEVEL_BITS) & ((1 << EXEC_PAGEMAP_LEVEL_BITS) - 1); + p2 = p1[pos]; + if (!p2) return EXEC_STATE_NONE; + + pos = addr & ((1 << EXEC_PAGEMAP_LEVEL_BITS) - 1); + return p2[pos]; +} + +static void register_as_executable(void *p, size_t len, int can_exec) +{ + exec_state_lock(); + while (len > 0) { + exec_pagemap_set(p, (can_exec ? EXEC_STATE_WRITE : EXEC_STATE_NONE)); + p = ((char *)p) + wx_page_size; + len -= wx_page_size; + } + exec_state_unlock(); +} + static void fault_handler(int sn, siginfo_t *si, void *ctx) { void *addr = si->si_addr; + exec_state_t es; int fail = 0; #ifdef MZ_PRECISE_GC @@ -1461,22 +1567,21 @@ static void fault_handler(int sn, siginfo_t *si, void *ctx) addr = (char *)addr - ((intptr_t)addr & (wx_page_size - 1)); - if (!addr) { - /* Always fail on the first page, such as for a NULL pointer - misuse */ + exec_state_lock(); + es = exec_pagemap_get(addr); + + if (es == EXEC_STATE_NONE) { fail = 1; - } else if (try_x) { - /* Maybe switching to execute mode will help. If not, we'll - get called again, and we'll try allowing writes */ + } else if (es == EXEC_STATE_WRITE) { + exec_pagemap_set(addr, EXEC_STATE_EXEC); if (mprotect(addr, wx_page_size, PROT_READ | PROT_EXEC)) fail = 1; - try_x = 0; } else { - /* Maybe switching to write mode will help... */ + exec_pagemap_set(addr, EXEC_STATE_WRITE); if (mprotect(addr, wx_page_size, PROT_READ | PROT_WRITE)) fail = 1; - try_x = 1; } + exec_state_unlock(); if (fail) { fprintf(stderr, "SIGSEGV at %p\n", si->si_addr); @@ -1484,9 +1589,22 @@ static void fault_handler(int sn, siginfo_t *si, void *ctx) } } +#ifdef OS_X +# define SIG_W_XOR_X SIGBUS +#else +# define SIG_W_XOR_X SIGSEGV +#endif + static void install_w_xor_x_handler() { wx_page_size = sysconf (_SC_PAGESIZE); + while (1 << wx_log_page_size < wx_page_size) + wx_log_page_size++; + +#ifdef MZ_USE_MZRT + mzrt_mutex_create(&exec_page_mutex); +#endif + { struct sigaction act, oact; memset(&act, 0, sizeof(act)); @@ -1495,9 +1613,10 @@ static void install_w_xor_x_handler() sigaddset(&act.sa_mask, SIGINT); sigaddset(&act.sa_mask, SIGCHLD); act.sa_flags = SA_SIGINFO; - sigaction(SIGSEGV, &act, &oact); + sigaction(SIG_W_XOR_X, &act, &oact); previous_fault_handler = oact.sa_sigaction; } + } #endif