From fb8447d59586d40fc8987ede4795ebda64354839 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Mon, 2 Jul 2012 02:06:29 -0400 Subject: [PATCH] Fix ticket #8650. - Replace arch_cpu_user_strlcpy() and arch_cpu_user_memset() with x86 assembly versions. These correctly handle the fault handler, which had broken again on gcc4 for the C versions, causing stack corruption in certain error cases. The other architectures will still need to have corresponding asm variants added in order for them to not run into the same issue though. --- src/system/kernel/arch/x86/arch_cpu.cpp | 60 +------------- src/system/kernel/arch/x86/arch_x86.S | 103 ++++++++++++++++++++++++ src/system/kernel/vm/vm.cpp | 4 +- 3 files changed, 107 insertions(+), 60 deletions(-) diff --git a/src/system/kernel/arch/x86/arch_cpu.cpp b/src/system/kernel/arch/x86/arch_cpu.cpp index 5ebb194bb1..8d44375dbf 100644 --- a/src/system/kernel/arch/x86/arch_cpu.cpp +++ b/src/system/kernel/arch/x86/arch_cpu.cpp @@ -4,6 +4,7 @@ * * Copyright 2001-2002, Travis Geiselbrecht. All rights reserved. * Distributed under the terms of the NewOS License. + * */ @@ -988,65 +989,6 @@ arch_cpu_invalidate_TLB_list(addr_t pages[], int num_pages) } -ssize_t -arch_cpu_user_strlcpy(char *to, const char *from, size_t size, - addr_t *faultHandler) -{ - int fromLength = 0; - addr_t oldFaultHandler = *faultHandler; - - // this check is to trick the gcc4 compiler and have it keep the error label - if (to == NULL && size > 0) - goto error; - - *faultHandler = (addr_t)&&error; - - if (size > 0) { - to[--size] = '\0'; - // copy - for ( ; size; size--, fromLength++, to++, from++) { - if ((*to = *from) == '\0') - break; - } - } - // count any leftover from chars - while (*from++ != '\0') { - fromLength++; - } - - *faultHandler = oldFaultHandler; - return fromLength; - -error: - *faultHandler = oldFaultHandler; - return B_BAD_ADDRESS; -} - - -status_t -arch_cpu_user_memset(void *s, char c, size_t count, addr_t *faultHandler) -{ - char *xs = (char *)s; - addr_t oldFaultHandler = *faultHandler; - - // this check is to trick the gcc4 compiler and have it keep the error label - if (s == NULL) - goto error; - - *faultHandler = (addr_t)&&error; - - while (count--) - *xs++ = c; - - *faultHandler = oldFaultHandler; - return 0; - -error: - *faultHandler = oldFaultHandler; - return B_BAD_ADDRESS; -} - - status_t arch_cpu_shutdown(bool rebootSystem) { diff --git a/src/system/kernel/arch/x86/arch_x86.S b/src/system/kernel/arch/x86/arch_x86.S index 28611fe247..58e20fc8cc 100644 --- a/src/system/kernel/arch/x86/arch_x86.S +++ b/src/system/kernel/arch/x86/arch_x86.S @@ -1,5 +1,6 @@ /* * Copyright 2003-2007, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2012, Rene Gollent, rene@gollent.com. * Distributed under the terms of the MIT License. * * Copyright 2001, Travis Geiselbrecht. All rights reserved. @@ -219,6 +220,108 @@ FUNCTION(arch_cpu_user_memcpy): FUNCTION_END(arch_cpu_user_memcpy) +/* status_t arch_cpu_user_memset(void *to, char c, size_t count, addr_t *faultHandler) */ +FUNCTION(arch_cpu_user_memset): + pushl %esi + pushl %edi + movl 12(%esp),%edi /* dest */ + movb 16(%esp),%al /* c */ + movl 20(%esp),%ecx /* count */ + + /* set the fault handler */ + movl 24(%esp),%edx /* fault handler */ + movl (%edx),%esi + movl $.L_user_memset_error, (%edx) + + rep + stosb + + /* restore the old fault handler */ + movl %esi,(%edx) + xor %eax,%eax + + popl %edi + popl %esi + ret + + /* error condition */ +.L_user_memset_error: + /* restore the old fault handler */ + movl %esi,(%edx) + movl $-1,%eax /* return a generic error, the wrapper routine will deal with it */ + popl %edi + popl %esi + ret +FUNCTION_END(arch_cpu_user_memset) + + +/* ssize_t arch_cpu_user_strlcpy(void *to, const void *from, size_t size, addr_t *faultHandler) */ +FUNCTION(arch_cpu_user_strlcpy): + pushl %esi + pushl %edi + pushl %ebx + movl 16(%esp),%edi /* dest */ + movl 20(%esp),%esi /* source */ + movl 24(%esp),%ecx /* count */ + + /* set the fault handler */ + movl 28(%esp),%edx /* fault handler */ + movl (%edx),%ebx + movl $.L_user_strlcpy_error, (%edx) + + /* Check for 0 length */ + cmp $0,%ecx + je .L_user_strlcpy_source_count + + /* Copy at most count - 1 bytes */ + dec %ecx + + /* move data by bytes */ + cld + repnz + movsb + + /* null terminate string */ + movb $0,(%edi) + dec %esi + + /* check if we copied the entire source string */ + cmp $0,%ecx + jne .L_user_strlcpy_source_done + + /* count remaining bytes in src */ +.L_user_strlcpy_source_count: + not %ecx + movb $0,%al + repnz + scasb + +.L_user_strlcpy_source_done: + + movl %esi,%eax + subl 20(%esp),%eax + subl $1,%eax + + /* restore the old fault handler */ + movl %ebx,(%edx) + + popl %ebx + popl %edi + popl %esi + ret + + /* error condition */ +.L_user_strlcpy_error: + /* restore the old fault handler */ + movl %ebx,(%edx) + movl $-1,%eax /* return a generic error, the wrapper routine will deal with it */ + popl %ebx + popl %edi + popl %esi + ret +FUNCTION_END(arch_cpu_user_strlcpy) + + /*! \fn void arch_debug_call_with_fault_handler(cpu_ent* cpu, jmp_buf jumpBuffer, void (*function)(void*), void* parameter) diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 857bf22808..e289def87f 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -5104,8 +5104,10 @@ user_strlcpy(char* to, const char* from, size_t size) &thread_get_current_thread()->fault_handler); // If we hit the address overflow boundary, fail. - if (result >= 0 && (size_t)result >= maxSize && maxSize < size) + if (result < 0 || (result >= 0 && (size_t)result >= maxSize + && maxSize < size)) { return B_BAD_ADDRESS; + } return result; }