From 8a71915a9df58aca2347b87f6ee4f7d60516786e Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Mon, 5 Oct 2009 09:54:40 +0000 Subject: [PATCH] Made the {debug,user}_{mem,strl}cpy() and user_memset() functions deal with address overflows. Apparently at least the x86 string instructions generate a general protection fault instead of a page fault, and we only use the fault handler in the latter case (maybe we should change that, too). Fixes #4714. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@33436 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/debug/debug.cpp | 22 +++++++++++++++++++++- src/system/kernel/vm/vm.cpp | 28 +++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/system/kernel/debug/debug.cpp b/src/system/kernel/debug/debug.cpp index 70679f056d..5cbb748421 100644 --- a/src/system/kernel/debug/debug.cpp +++ b/src/system/kernel/debug/debug.cpp @@ -13,6 +13,8 @@ #include "blue_screen.h" +#include + #include #include #include @@ -1529,6 +1531,10 @@ debug_call_with_fault_handler(jmp_buf jumpBuffer, void (*function)(void*), status_t debug_memcpy(void* to, const void* from, size_t size) { + // don't allow address overflows + if ((addr_t)from + size < (addr_t)from || (addr_t)to + size < (addr_t)to) + return B_BAD_ADDRESS; + debug_memcpy_parameters parameters = {to, from, size}; if (debug_call_with_fault_handler(gCPU[sDebuggerOnCPU].fault_jump_buffer, @@ -1545,12 +1551,26 @@ debug_memcpy(void* to, const void* from, size_t size) ssize_t debug_strlcpy(char* to, const char* from, size_t size) { - debug_strlcpy_parameters parameters = {to, from, size}; + if (size == 0) + return 0; + if (from == NULL || to == NULL) + return B_BAD_ADDRESS; + + // limit size to avoid address overflows + size_t maxSize = std::min(size, + ~(addr_t)0 - std::max((addr_t)from, (addr_t)to) + 1); + + debug_strlcpy_parameters parameters = {to, from, maxSize}; if (debug_call_with_fault_handler(gCPU[sDebuggerOnCPU].fault_jump_buffer, &debug_strlcpy_trampoline, ¶meters) != 0) { return B_BAD_ADDRESS; } + + // If we hit the address overflow boundary, fail. + if (parameters.result == maxSize && maxSize < size) + return B_BAD_ADDRESS; + return parameters.result; } diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index c71c1aa579..d45c0671b4 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -16,6 +16,8 @@ #include #include +#include + #include #include @@ -5457,6 +5459,10 @@ vm_memcpy_physical_page(addr_t to, addr_t from) status_t user_memcpy(void* to, const void* from, size_t size) { + // don't allow address overflows + if ((addr_t)from + size < (addr_t)from || (addr_t)to + size < (addr_t)to) + return B_BAD_ADDRESS; + if (arch_cpu_user_memcpy(to, from, size, &thread_get_current_thread()->fault_handler) < B_OK) return B_BAD_ADDRESS; @@ -5477,14 +5483,34 @@ user_memcpy(void* to, const void* from, size_t size) ssize_t user_strlcpy(char* to, const char* from, size_t size) { - return arch_cpu_user_strlcpy(to, from, size, + if (size == 0) + return 0; + if (from == NULL || to == NULL) + return B_BAD_ADDRESS; + + // limit size to avoid address overflows + size_t maxSize = std::min(size, + ~(addr_t)0 - std::max((addr_t)from, (addr_t)to) + 1); + + + ssize_t result = arch_cpu_user_strlcpy(to, from, maxSize, &thread_get_current_thread()->fault_handler); + + // If we hit the address overflow boundary, fail. + if (result >= 0 && (size_t)result == maxSize && maxSize < size) + return B_BAD_ADDRESS; + + return result; } status_t user_memset(void* s, char c, size_t count) { + // don't allow address overflows + if ((addr_t)s + count < (addr_t)s) + return B_BAD_ADDRESS; + if (arch_cpu_user_memset(s, c, count, &thread_get_current_thread()->fault_handler) < B_OK) return B_BAD_ADDRESS;