From 07cfddfa0c7eede8fee6c02cdb282c07db351b75 Mon Sep 17 00:00:00 2001
From: Augustin Cavalier <waddlesplash@gmail.com>
Date: Fri, 30 Aug 2024 16:40:01 -0400
Subject: [PATCH] kernel/x86: Rework handling of %edx in syscall handler
 (again.)

The previous change to this logic (9921f444625e360674f92db84ffae695492233a5)
apparently caused intermittent crashes with various applications. It
seems that EDX is not expected to be clobbered by functions that do not have 64-bit return values, and if it is, crashes result, like #19024.

This commit reworks the logic to not change %edx in the iframe at all
if we don't have a 64-bit return value, and then adjusts the debug logic
to clear %edx before invoking the post-syscall debugger hook. This means
we have to run the post-syscall debugger hook before clearing the
flags, but that should be fine (and perhaps even useful.)

Fixes #19024, and possibly other crashes that have cropped up on
32-bit x86 in the last few weeks, at least.

Change-Id: I3280033bc2dd05aca254555d6ee3b173a270ebf9
Reviewed-on: https://review.haiku-os.org/c/haiku/+/8158
Haiku-Format: Haiku-format Bot <no-reply+haikuformatbot@haiku-os.org>
Reviewed-by: Michael Lotz <mmlr@mlotz.ch>
---
 src/system/kernel/arch/x86/32/interrupts.S | 50 ++++++++++++----------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/src/system/kernel/arch/x86/32/interrupts.S b/src/system/kernel/arch/x86/32/interrupts.S
index 2473e89eff..9f79ad6269 100644
--- a/src/system/kernel/arch/x86/32/interrupts.S
+++ b/src/system/kernel/arch/x86/32/interrupts.S
@@ -172,6 +172,10 @@
 		addl	$8, %esp;
 
 #	define TRACE_POST_SYSCALL()	\
+		testl	$THREAD_FLAGS_64_BIT_SYSCALL_RETURN, THREAD_flags(%edi);	\
+		jnz		1f;															\
+		xor		%edx, %edx;													\
+1:																			\
 		push	%edx;							/* syscall return value */	\
 		push	%eax;														\
 		movl	IFRAME_orig_eax(%ebp), %eax;	/* syscall number */		\
@@ -642,13 +646,11 @@ STATIC_FUNCTION(handle_syscall):
 	// call the syscall function
 	call	*SYSCALL_INFO_function(%esi)
 
-	testl	$THREAD_FLAGS_64_BIT_SYSCALL_RETURN, THREAD_flags(%edi)
-	jnz		1f
-	xor		%edx, %edx
-  1:
-
 	// overwrite the values of %eax and %edx on the stack (the syscall return value)
+	testl	$THREAD_FLAGS_64_BIT_SYSCALL_RETURN, THREAD_flags(%edi)
+	jz		1f
 	movl	%edx, IFRAME_dx(%ebp)
+  1:
 	movl	%eax, IFRAME_ax(%ebp)
 
 	TRACE_POST_SYSCALL()
@@ -682,6 +684,27 @@ FUNCTION_END(handle_syscall)
   FUNCTION_END(do_pre_syscall_debug)
 
   STATIC_FUNCTION(post_syscall_work):
+	// post syscall debugging
+	testl	$THREAD_FLAGS_DEBUGGER_INSTALLED, THREAD_flags(%edi)
+	jz		2f
+	pushl	-8(%ebp)				// syscall start time
+	pushl	-12(%ebp)
+	xor		%edx, %edx
+	testl	$THREAD_FLAGS_64_BIT_SYSCALL_RETURN, THREAD_flags(%edi)
+	jz		1f
+	movl	IFRAME_dx(%ebp), %edx	// syscall return value
+  1:
+	movl	IFRAME_ax(%ebp), %eax
+	push	%edx
+	push	%eax
+	lea		16(%esp), %eax			// syscall parameters
+	push	%eax
+	movl	IFRAME_orig_eax(%ebp), %eax		// syscall number
+	push	%eax
+	call	user_debug_post_syscall
+	addl	$24, %esp
+
+  2:
 	// clear the 64 bit return value and syscall restarted bits
 	testl	$(THREAD_FLAGS_64_BIT_SYSCALL_RETURN \
 				| THREAD_FLAGS_SYSCALL_RESTARTED), THREAD_flags(%edi)
@@ -695,23 +718,6 @@ FUNCTION_END(handle_syscall)
 	cmpxchgl	%edx, THREAD_flags(%edi)
 	jnz		1b
   2:
-
-	// post syscall debugging
-	testl	$THREAD_FLAGS_DEBUGGER_INSTALLED, THREAD_flags(%edi)
-	jz		1f
-	pushl	-8(%ebp)				// syscall start time
-	pushl	-12(%ebp)
-	movl	IFRAME_dx(%ebp), %edx	// syscall return value
-	movl	IFRAME_ax(%ebp), %eax
-	push	%edx
-	push	%eax
-	lea		16(%esp), %eax			// syscall parameters
-	push	%eax
-	movl	IFRAME_orig_eax(%ebp), %eax		// syscall number
-	push	%eax
-	call	user_debug_post_syscall
-	addl	$24, %esp
-  1:
   FUNCTION_END(post_syscall_work)
 
   bad_syscall_number: