hashx: allow hashx_compile to fail, avoid segfault without changing API

This is a minimal portion of the fix for tor issue #40794, in which
hashx segfaults due to denial of mprotect() syscalls at runtime.

Prior to this fix, hashx makes the assumption that if the JIT is
supported on the current architecture, it will also be usable at
runtime. This isn't true if mprotect fails on linux, which it may for
various reasons: the tor built-in sandbox, the shadow simulator, or
external security software that implements a syscall filter.

The necessary error propagation was missing internally in hashx,
causing us to obliviously call into code which was never made
executable. With this fix, hashx_make() will instead fail by returning
zero.

A proper fix will require API changes so that callers can discern
between different types of failures. Zero already means that a program
couldn't be constructed, which requires a different response: choosing a
different seed, vs switching implementations. Callers would also benefit
from a way to use one context (with its already-built program) to
run in either compiled or interpreted mode.

Signed-off-by: Micah Elizabeth Scott <beth@torproject.org>

diff --git a/src/ext/equix/hashx/src/compiler.h b/src/ext/equix/hashx/src/compiler.h
index a100806fea075258dba7565d66b2c51b3c64aba0..140f797a586ea16e15954153f6c78baeac388d98 100644 (file)
--- a/src/ext/equix/hashx/src/compiler.h
+++ b/src/ext/equix/hashx/src/compiler.h
@@ -10,9 +10,9 @@
 #include "virtual_memory.h"
 #include "program.h"
 
-HASHX_PRIVATE void hashx_compile_x86(const hashx_program* program, uint8_t* code);
+HASHX_PRIVATE bool hashx_compile_x86(const hashx_program* program, uint8_t* code);
 
-HASHX_PRIVATE void hashx_compile_a64(const hashx_program* program, uint8_t* code);
+HASHX_PRIVATE bool hashx_compile_a64(const hashx_program* program, uint8_t* code);
 
 #if defined(_M_X64) || defined(__x86_64__)
 #define HASHX_COMPILER 1
@@ -24,7 +24,7 @@ HASHX_PRIVATE void hashx_compile_a64(const hashx_program* program, uint8_t* code
 #define hashx_compile(p,c) hashx_compile_a64(p,c)
 #else
 #define HASHX_COMPILER 0
-#define hashx_compile(p,c)
+#define hashx_compile(p,c) (false)
 #endif
 
 HASHX_PRIVATE bool hashx_compiler_init(hashx_ctx* compiler);

diff --git a/src/ext/equix/hashx/src/compiler_a64.c b/src/ext/equix/hashx/src/compiler_a64.c
index 48f743b9883beca7ed2bf9a385ef640bb49b6959..94635ad1b7d36e12b69769f231be2d4d8bc7ac84 100644 (file)
--- a/src/ext/equix/hashx/src/compiler_a64.c
+++ b/src/ext/equix/hashx/src/compiler_a64.c
@@ -48,8 +48,9 @@ static const uint8_t a64_epilogue[] = {
        0xc0, 0x03, 0x5f, 0xd6, /* ret               */
 };
 
-void hashx_compile_a64(const hashx_program* program, uint8_t* code) {
-       hashx_vm_rw(code, COMP_CODE_SIZE);
+bool hashx_compile_a64(const hashx_program* program, uint8_t* code) {
+       if (!hashx_vm_rw(code, COMP_CODE_SIZE))
+               return false;
        uint8_t* pos = code;
        uint8_t* target = NULL;
        int creg = -1;
@@ -145,10 +146,12 @@ void hashx_compile_a64(const hashx_program* program, uint8_t* code) {
                }
        }
        EMIT(pos, a64_epilogue);
-       hashx_vm_rx(code, COMP_CODE_SIZE);
+       if (!hashx_vm_rx(code, COMP_CODE_SIZE))
+               return false;
 #ifdef __GNUC__
        __builtin___clear_cache(code, pos);
 #endif
+       return true;
 }
 
 #endif

diff --git a/src/ext/equix/hashx/src/compiler_x86.c b/src/ext/equix/hashx/src/compiler_x86.c
index f03b17cca410b7cd03fe072e94d062cfae46815d..12f59a1d0b64e39458fae62ec5d7d4fb775d3ec5 100644 (file)
--- a/src/ext/equix/hashx/src/compiler_x86.c
+++ b/src/ext/equix/hashx/src/compiler_x86.c
@@ -81,8 +81,9 @@ static const uint8_t x86_epilogue[] = {
        0xC3                          /* ret */
 };
 
-void hashx_compile_x86(const hashx_program* program, uint8_t* code) {
-       hashx_vm_rw(code, COMP_CODE_SIZE);
+bool hashx_compile_x86(const hashx_program* program, uint8_t* code) {
+       if (!hashx_vm_rw(code, COMP_CODE_SIZE))
+               return false;
        uint8_t* pos = code;
        uint8_t* target = NULL;
        EMIT(pos, x86_prologue);
@@ -145,7 +146,7 @@ void hashx_compile_x86(const hashx_program* program, uint8_t* code) {
                }
        }
        EMIT(pos, x86_epilogue);
-       hashx_vm_rx(code, COMP_CODE_SIZE);
+       return hashx_vm_rx(code, COMP_CODE_SIZE);
 }
 
 #endif

diff --git a/src/ext/equix/hashx/src/hashx.c b/src/ext/equix/hashx/src/hashx.c
index da84aa51f36951a3633d33f1223a6a9e175f12ac..372df09581efa95c50761355a80b0da5dce5ed44 100644 (file)
--- a/src/ext/equix/hashx/src/hashx.c
+++ b/src/ext/equix/hashx/src/hashx.c
@@ -64,7 +64,9 @@ int hashx_make(hashx_ctx* ctx, const void* seed, size_t size) {
                if (!initialize_program(ctx, &program, keys)) {
                        return 0;
                }
-               hashx_compile(&program, ctx->code);
+               if (!hashx_compile(&program, ctx->code)) {
+                       return 0;
+               }
                return 1;
        }
        return initialize_program(ctx, ctx->program, keys);

diff --git a/src/ext/equix/hashx/src/virtual_memory.c b/src/ext/equix/hashx/src/virtual_memory.c
index e01fd878b969d958e55f9641974e6dcd158a31af..8d574740c59b58085364f5e9f24a8f7d5c498546 100644 (file)
--- a/src/ext/equix/hashx/src/virtual_memory.c
+++ b/src/ext/equix/hashx/src/virtual_memory.c
@@ -22,7 +22,7 @@
 
 #ifdef HASHX_WIN
 
-static int set_privilege(const char* pszPrivilege, BOOL bEnable) {
+static bool set_privilege(const char* pszPrivilege, BOOL bEnable) {
        HANDLE           hToken;
        TOKEN_PRIVILEGES tp;
        BOOL             status;
@@ -30,10 +30,10 @@ static int set_privilege(const char* pszPrivilege, BOOL bEnable) {
 
        if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES
                | TOKEN_QUERY, &hToken))
-               return 0;
+               return false;
 
        if (!LookupPrivilegeValue(NULL, pszPrivilege, &tp.Privileges[0].Luid))
-               return 0;
+               return false;
 
        tp.PrivilegeCount = 1;
 
@@ -64,31 +64,33 @@ void* hashx_vm_alloc(size_t bytes) {
        return mem;
 }
 
-static inline int page_protect(void* ptr, size_t bytes, int rules) {
+static inline bool page_protect(void* ptr, size_t bytes, int rules) {
 #ifdef HASHX_WIN
        DWORD oldp;
        if (!VirtualProtect(ptr, bytes, (DWORD)rules, &oldp)) {
-               return 0;
+               return false;
        }
 #else
-       if (-1 == mprotect(ptr, bytes, rules))
-               return 0;
+       if (mprotect(ptr, bytes, rules) != 0)
+               return false;
 #endif
-       return 1;
+       return true;
 }
 
-void hashx_vm_rw(void* ptr, size_t bytes) {
-       page_protect(ptr, bytes, PAGE_READWRITE);
+bool hashx_vm_rw(void* ptr, size_t bytes) {
+       return page_protect(ptr, bytes, PAGE_READWRITE);
 }
 
-void hashx_vm_rx(void* ptr, size_t bytes) {
-       page_protect(ptr, bytes, PAGE_EXECUTE_READ);
+bool hashx_vm_rx(void* ptr, size_t bytes) {
+       return page_protect(ptr, bytes, PAGE_EXECUTE_READ);
 }
 
 void* hashx_vm_alloc_huge(size_t bytes) {
        void* mem;
 #ifdef HASHX_WIN
-       set_privilege("SeLockMemoryPrivilege", 1);
+       if (!set_privilege("SeLockMemoryPrivilege", 1)) {
+               /* Failed, but try the VirtualAlloc anyway */
+       }
        SIZE_T page_min = GetLargePageMinimum();
        if (page_min > 0) {
                mem = VirtualAlloc(NULL, ALIGN_SIZE(bytes, page_min), MEM_COMMIT

diff --git a/src/ext/equix/hashx/src/virtual_memory.h b/src/ext/equix/hashx/src/virtual_memory.h
index d08f74dcc63bf349455031869ab4b40f7b310c2e..9780d218c2a41fa9adf8af337f2feed906565790 100644 (file)
--- a/src/ext/equix/hashx/src/virtual_memory.h
+++ b/src/ext/equix/hashx/src/virtual_memory.h
@@ -6,13 +6,14 @@
 
 #include <stdint.h>
 #include <stddef.h>
+#include <stdbool.h>
 #include <hashx.h>
 
 #define ALIGN_SIZE(pos, align) ((((pos) - 1) / (align) + 1) * (align))
 
 HASHX_PRIVATE void* hashx_vm_alloc(size_t size);
-HASHX_PRIVATE void hashx_vm_rw(void* ptr, size_t size);
-HASHX_PRIVATE void hashx_vm_rx(void* ptr, size_t size);
+HASHX_PRIVATE bool hashx_vm_rw(void* ptr, size_t size);
+HASHX_PRIVATE bool hashx_vm_rx(void* ptr, size_t size);
 HASHX_PRIVATE void* hashx_vm_alloc_huge(size_t size);
 HASHX_PRIVATE void hashx_vm_free(void* ptr, size_t size);