x86: move x86_userspace_thread_exit() from user stack to commpage

x86_userspace_thread_exit() is a stub originally placed at the bottom of
each thread user stack that ensures any thread invokes exit_thread() upon
returning from its main higher level function.

Putting anything that is expected to be executed on a stack causes problems
when implementing data execution prevention. Code of x86_userspace_thread_exit()
is now moved to commpage which seems to be much more appropriate place for it.

headers/private/system/arch/x86/arch_commpage_defs.h

@@ -16,6 +16,8 @@
 									(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 3)
 #define COMMPAGE_ENTRY_X86_SIGNAL_HANDLER_BEOS \
 									(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 4)
+#define COMMPAGE_ENTRY_X86_THREAD_EXIT \
+									(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 5)
 
 #define ARCH_USER_COMMPAGE_ADDR (0xffff0000)
 

headers/private/system/arch/x86_64/arch_commpage_defs.h

@@ -13,6 +13,8 @@
 #define COMMPAGE_ENTRY_X86_MEMSET	(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 1)
 #define COMMPAGE_ENTRY_X86_SIGNAL_HANDLER \
 									(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 2)
+#define COMMPAGE_ENTRY_X86_THREAD_EXIT \
+									(COMMPAGE_ENTRY_FIRST_ARCH_SPECIFIC + 3)
 
 #define ARCH_USER_COMMPAGE_ADDR		(0xffffffffffff0000)
 

src/system/kernel/arch/x86/32/arch.S

@@ -115,7 +115,7 @@ FUNCTION(x86_swap_pgdir):
 	ret
 FUNCTION_END(x86_swap_pgdir)
 
-/* thread exit stub - is copied to the userspace stack in arch_thread_enter_uspace() */
+/* thread exit stub */
 	.align 4
 FUNCTION(x86_userspace_thread_exit):
 	pushl	%eax

src/system/kernel/arch/x86/32/thread.cpp

@@ -13,6 +13,7 @@
 
 #include <arch/user_debugger.h>
 #include <arch_cpu.h>
+#include <commpage.h>
 #include <cpu.h>
 #include <debug.h>
 #include <kernel.h>
@@ -215,8 +216,6 @@ arch_thread_enter_userspace(Thread* thread, addr_t entry, void* args1,
 	void* args2)
 {
 	addr_t stackTop = thread->user_stack_base + thread->user_stack_size;
-	uint32 codeSize = (addr_t)x86_end_userspace_thread_exit
-		- (addr_t)x86_userspace_thread_exit;
 	uint32 args[3];
 
 	TRACE(("arch_thread_enter_userspace: entry 0x%lx, args %p %p, "
@@ -224,14 +223,11 @@ arch_thread_enter_userspace(Thread* thread, addr_t entry, void* args1,
 
 	stackTop = arch_randomize_stack_pointer(stackTop);
 
-	// copy the little stub that calls exit_thread() when the thread entry
+	// Copy the address of the stub that calls exit_thread() when the thread
-	// function returns, as well as the arguments of the entry function
+	// entry function returns to the top of the stack to act as the return
-	stackTop -= codeSize;
+	// address. The stub is inside commpage.
-
+	args[0] = *(addr_t*)(USER_COMMPAGE_ADDR
-	if (user_memcpy((void *)stackTop, (const void *)&x86_userspace_thread_exit, codeSize) < B_OK)
+		+ COMMPAGE_ENTRY_X86_THREAD_EXIT * sizeof(addr_t));
-		return B_BAD_ADDRESS;
-
-	args[0] = stackTop;
 	args[1] = (uint32)args1;
 	args[2] = (uint32)args2;
 	stackTop -= sizeof(args);

src/system/kernel/arch/x86/64/arch.S

@@ -118,7 +118,7 @@ FUNCTION(x86_swap_pgdir):
 FUNCTION_END(x86_swap_pgdir)
 
 
-/* thread exit stub - copied to the userspace stack in arch_thread_enter_uspace() */
+/* thread exit stub */
 .align 8
 FUNCTION(x86_userspace_thread_exit):
 	movq	%rax, %rdi

src/system/kernel/arch/x86/64/thread.cpp

@@ -218,20 +218,11 @@ arch_thread_enter_userspace(Thread* thread, addr_t entry, void* args1,
 
 	stackTop = arch_randomize_stack_pointer(stackTop);
 
-	// Copy the little stub that calls exit_thread() when the thread entry
+	// Copy the address of the stub that calls exit_thread() when the thread
-	// function returns.
+	// entry function returns to the top of the stack to act as the return
-	// TODO: This will become a problem later if we want to support execute
+	// address. The stub is inside commpage.
-	// disable, the stack shouldn't really be executable.
+	addr_t codeAddr = *(addr_t*)(USER_COMMPAGE_ADDR
-	size_t codeSize = (addr_t)x86_end_userspace_thread_exit
+		+ COMMPAGE_ENTRY_X86_THREAD_EXIT * sizeof(addr_t));
-		- (addr_t)x86_userspace_thread_exit;
-	stackTop -= codeSize;
-	if (user_memcpy((void*)stackTop, (const void*)&x86_userspace_thread_exit,
-			codeSize) != B_OK)
-		return B_BAD_ADDRESS;
-
-	// Copy the address of the stub to the top of the stack to act as the
-	// return address.
-	addr_t codeAddr = stackTop;
 	stackTop -= sizeof(codeAddr);
 	if (user_memcpy((void*)stackTop, (const void*)&codeAddr, sizeof(codeAddr))
 			!= B_OK)

src/system/kernel/arch/x86/arch_cpu.cpp

@@ -868,6 +868,10 @@ arch_cpu_init_post_modules(kernel_args* args)
 		- (addr_t)gOptimizedFunctions.memset;
 	fill_commpage_entry(COMMPAGE_ENTRY_X86_MEMSET,
 		(const void*)gOptimizedFunctions.memset, memsetLen);
+	size_t threadExitLen = (addr_t)x86_end_userspace_thread_exit
+		- (addr_t)x86_userspace_thread_exit;
+	fill_commpage_entry(COMMPAGE_ENTRY_X86_THREAD_EXIT,
+		(const void*)x86_userspace_thread_exit, threadExitLen);
 
 	// add the functions to the commpage image
 	image_id image = get_commpage_image();
@@ -877,6 +881,9 @@ arch_cpu_init_post_modules(kernel_args* args)
 	elf_add_memory_image_symbol(image, "commpage_memset",
 		((addr_t*)USER_COMMPAGE_ADDR)[COMMPAGE_ENTRY_X86_MEMSET], memsetLen,
 		B_SYMBOL_TYPE_TEXT);
+	elf_add_memory_image_symbol(image, "commpage_thread_exit",
+		((addr_t*)USER_COMMPAGE_ADDR)[COMMPAGE_ENTRY_X86_THREAD_EXIT],
+		threadExitLen, B_SYMBOL_TYPE_TEXT);
 
 	return B_OK;
 }