i386 entry.S problems

["Jan Beulich" <JBeulich@novell.com>]

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

There appear to be two problems in i386's entry.S:

(1) With CONFIG_REGPARM, lcall7 and lcall27 did not work (they pass the
parameters to the actual handler procedure on the stack). While using
'asmlinkage' on the handler function would have seemed feasible (leaving
aside binary compatibility issues), it not really is because this is a
pointer to a function and gcc does not check that parameter passing
options match for function pointer assignments, thus making it
impossible to detect improper uses.

(2) In the NMI handler, the second of the checks for whether the
interrupted code was in the stack pointer checking code of the debug
fault/trap handler was reversed, leading to the respective fixup getting
applied whenever the interrupted code was at an address higher than the
debug fault/trap handler.

The patch attempts to address both.

Additionally it seems to me that the code early in the NMI handler that
supposedly checks whether the stack pointer is in range for the
subsequent accesses to the outer stack frame is broken, too. Namely, it
uses THREAD_SIZE to check a pointer that actually follows immediately
the TSS and is not even a single page in size, and is also not
THREAD_SIZE aligned. Thus this test is very likely to fail depending on
the exact address the individual CPU's init_tss ends up on (one may even
say guaranteed, since init_tss should in most configurations live
directly after idt_table and thus at 0x800 bytes into a page). I can't
immediately see a correct replacement for the existing code.

Jan

--- linux-2.6.8.1/arch/i386/kernel/entry.S.0	2004-08-14
12:55:09.000000000 +0200
+++ linux-2.6.8.1/arch/i386/kernel/entry.S	2004-09-24
15:25:13.968031736 +0200
@@ -147,7 +147,9 @@ ENTRY(lcall7)
 	pushl %eax
 	SAVE_ALL
 	movl %esp, %ebp
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
 	pushl %ebp
+#endif
 	pushl $0x7
 do_lcall:
 	movl EIP(%ebp), %eax	# due to call gates, this is eflags, not
eip..
@@ -156,11 +158,18 @@ do_lcall:
 	movl %eax,EFLAGS(%ebp)	#
 	movl %edx,EIP(%ebp)	# Now we move them to their "normal"
places
 	movl %ecx,CS(%ebp)	#
-	GET_THREAD_INFO_WITH_ESP(%ebp)	# GET_THREAD_INFO
-	movl TI_exec_domain(%ebp), %edx	# Get the execution domain
-	call *EXEC_DOMAIN_handler(%edx)	# Call the handler for the
domain
-	addl $4, %esp
+#if defined(CONFIG_REGPARM) && __GNUC__ >= 3
 	popl %eax
+#endif
+	GET_THREAD_INFO_WITH_ESP(%ebp)	# GET_THREAD_INFO
+	movl TI_exec_domain(%ebp), %ecx	# Get the execution domain
+#if defined(CONFIG_REGPARM) && __GNUC__ >= 3
+	movl %esp, %edx
+#endif
+	call *EXEC_DOMAIN_handler(%ecx)	# Call the handler for the
domain
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
+	addl $8, %esp
+#endif
 	jmp resume_userspace
 
 ENTRY(lcall27)
@@ -169,7 +178,9 @@ ENTRY(lcall27)
 	pushl %eax
 	SAVE_ALL
 	movl %esp, %ebp
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
 	pushl %ebp
+#endif
 	pushl $0x27
 	jmp do_lcall
 
@@ -531,10 +542,10 @@ nmi_stack_fixup:
 nmi_debug_stack_check:
 	cmpw $__KERNEL_CS,16(%esp)
 	jne nmi_stack_correct
-	cmpl $debug - 1,(%esp)
-	jle nmi_stack_correct
+	cmpl $debug,(%esp)
+	jb nmi_stack_correct
 	cmpl $debug_esp_fix_insn,(%esp)
-	jle nmi_debug_stack_fixup
+	ja nmi_stack_correct
 nmi_debug_stack_fixup:
 	FIX_STACK(24,nmi_stack_correct, 1)
 	jmp nmi_stack_correct


[-- Attachment #2: linux-2.6.8.1-i386-entry.patch --]
[-- Type: application/octet-stream, Size: 1724 bytes --]

--- linux-2.6.8.1/arch/i386/kernel/entry.S.0	2004-08-14 12:55:09.000000000 +0200
+++ linux-2.6.8.1/arch/i386/kernel/entry.S	2004-09-24 15:25:13.968031736 +0200
@@ -147,7 +147,9 @@ ENTRY(lcall7)
 	pushl %eax
 	SAVE_ALL
 	movl %esp, %ebp
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
 	pushl %ebp
+#endif
 	pushl $0x7
 do_lcall:
 	movl EIP(%ebp), %eax	# due to call gates, this is eflags, not eip..
@@ -156,11 +158,18 @@ do_lcall:
 	movl %eax,EFLAGS(%ebp)	#
 	movl %edx,EIP(%ebp)	# Now we move them to their "normal" places
 	movl %ecx,CS(%ebp)	#
-	GET_THREAD_INFO_WITH_ESP(%ebp)	# GET_THREAD_INFO
-	movl TI_exec_domain(%ebp), %edx	# Get the execution domain
-	call *EXEC_DOMAIN_handler(%edx)	# Call the handler for the domain
-	addl $4, %esp
+#if defined(CONFIG_REGPARM) && __GNUC__ >= 3
 	popl %eax
+#endif
+	GET_THREAD_INFO_WITH_ESP(%ebp)	# GET_THREAD_INFO
+	movl TI_exec_domain(%ebp), %ecx	# Get the execution domain
+#if defined(CONFIG_REGPARM) && __GNUC__ >= 3
+	movl %esp, %edx
+#endif
+	call *EXEC_DOMAIN_handler(%ecx)	# Call the handler for the domain
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
+	addl $8, %esp
+#endif
 	jmp resume_userspace
 
 ENTRY(lcall27)
@@ -169,7 +178,9 @@ ENTRY(lcall27)
 	pushl %eax
 	SAVE_ALL
 	movl %esp, %ebp
+#if !defined(CONFIG_REGPARM) || __GNUC__ < 3
 	pushl %ebp
+#endif
 	pushl $0x27
 	jmp do_lcall
 
@@ -531,10 +542,10 @@ nmi_stack_fixup:
 nmi_debug_stack_check:
 	cmpw $__KERNEL_CS,16(%esp)
 	jne nmi_stack_correct
-	cmpl $debug - 1,(%esp)
-	jle nmi_stack_correct
+	cmpl $debug,(%esp)
+	jb nmi_stack_correct
 	cmpl $debug_esp_fix_insn,(%esp)
-	jle nmi_debug_stack_fixup
+	ja nmi_stack_correct
 nmi_debug_stack_fixup:
 	FIX_STACK(24,nmi_stack_correct, 1)
 	jmp nmi_stack_correct