This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 6fd5ca491405807c0ac92ff9f343377b1c11a5c8 Author: Micah Elizabeth Scott beth@torproject.org AuthorDate: Wed May 24 13:25:54 2023 -0700
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 --- src/ext/equix/hashx/src/compiler.h | 6 +++--- src/ext/equix/hashx/src/compiler_a64.c | 9 ++++++--- src/ext/equix/hashx/src/compiler_x86.c | 7 ++++--- src/ext/equix/hashx/src/hashx.c | 4 +++- src/ext/equix/hashx/src/virtual_memory.c | 28 +++++++++++++++------------- src/ext/equix/hashx/src/virtual_memory.h | 5 +++-- 6 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/src/ext/equix/hashx/src/compiler.h b/src/ext/equix/hashx/src/compiler.h index a100806fea..140f797a58 100644 --- 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 48f743b988..94635ad1b7 100644 --- 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 f03b17cca4..12f59a1d0b 100644 --- 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 da84aa51f3..372df09581 100644 --- 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 e01fd878b9..8d574740c5 100644 --- 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 d08f74dcc6..9780d218c2 100644 --- 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);