[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor] 03/07: hashx: allow hashx_compile to fail, avoid segfault without changing API



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@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
---
 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);
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits