[RFC] [PATCH] add TRAP_BAD_SYSCALL_EXITS config for i386

[RFC] [PATCH] add TRAP_BAD_SYSCALL_EXITS config for i386

[Andy Whitcroft]

There seems to be code recently added to -bk and thereby -mm which supports
extra debug for preempt on system call exit.  Oddly there doesn't seem
to be configuration options to enable them.  Below is a possible patch
to allow enabling this on i386.  Sadly the most obvious menu to add this
to is the Kernel Hacking menu, but that is defined in architecture specific
configuration.  If this makes sense I could patch the other arches?

Comments?

-apw

=== 8< ===
Add a configuration option to allow enabling TRAP_BAD_SYSCALL_EXITS to the
Kernel Hacking menu.

Revision: $Rev: 356 $

Signed-off-by: Andy Whitcroft <apw@shadowen.org>

---
 Kconfig |    7 +++++++
 1 files changed, 7 insertions(+)

diff -upN reference/arch/i386/Kconfig current/arch/i386/Kconfig
--- reference/arch/i386/Kconfig	2004-07-02 14:00:51.000000000 +0100
+++ current/arch/i386/Kconfig	2004-07-02 16:40:49.000000000 +0100
@@ -1492,6 +1492,13 @@ config X86_MPPARSE
 	depends on X86_LOCAL_APIC && !X86_VISWS
 	default y
 
+config TRAP_BAD_SYSCALL_EXITS
+	bool "Debug bad system call exits"
+	help
+	  If you say Y here the kernel will check for system calls which
+	  return without clearing preempt.
+        default n
+
 endmenu
 
 source "security/Kconfig"

Re: [RFC] [PATCH] add TRAP_BAD_SYSCALL_EXITS config for i386

[Andrew Morton]

Andy Whitcroft <apw@shadowen.org> wrote:
>
> There seems to be code recently added to -bk and thereby -mm which supports
> extra debug for preempt on system call exit.  Oddly there doesn't seem
> to be configuration options to enable them.  Below is a possible patch
> to allow enabling this on i386.  Sadly the most obvious menu to add this
> to is the Kernel Hacking menu, but that is defined in architecture specific
> configuration.  If this makes sense I could patch the other arches?

The TRAP_BAD_SYSCALL stuff is actually a bloa^Wfeature which was added
via the kgdb patch, so it is not in -bk.

I've never used it, dunno what it does.  I'll roll your two patches into the
kgdb patches in -mm, thanks.

Re: [RFC] [PATCH] add TRAP_BAD_SYSCALL_EXITS config for i386

[Andy Whitcroft]

> The TRAP_BAD_SYSCALL stuff is actually a bloa^Wfeature which was added
> via the kgdb patch, so it is not in -bk.
> 
> I've never used it, dunno what it does.  I'll roll your two patches into the
> kgdb patches in -mm, thanks.

This adds code to the syscall return path to check that we are
not returning with preempt_count() != 0.  I think that this is a
pretty useful diagnostics tool.  I think that this part should be
split off and considered for inclusion separatly from the GBD part.
It seems that they intended to cause a breakpoint when this occurs.
The small assembly stub combined with something like the patch
below would stand alone.  I've used it as a diagnotics tool.

I could put together a patch to separate this functionality off
from the GDB patches.  If you agree its worthwhile I am happy to
talk to the GDB people about it.

-apw

=== 8< ===
When we detect that a system call has returned with preempt still disabled
report this situation, including the system call number, return value and
preempt value, as well a dropping a register dump.  In the spirit of other
oops handling attempt to recover from it and continue.

Revision: $Rev: 371 $

Signed-off-by: Andy Whitcroft <apw@shadowen.org>

---

diff -X /home/apw/brief/lib/vdiff.excl -rupN reference/arch/i386/kernel/entry.S current/arch/i386/kernel/entry.S
--- reference/arch/i386/kernel/entry.S	2004-07-07 14:34:58.000000000 +0100
+++ current/arch/i386/kernel/entry.S	2004-07-07 14:40:19.000000000 +0100
@@ -317,7 +317,13 @@ restore_all:
 	cmpl $0,TI_preempt_count(%ebp)  # non-zero preempt_count ?
 	jz resume_kernelX
 
-        int $3
+        movl %esp, %ebx			# Record the original register dump
+	movl ORIG_EAX(%esp), %eax	# Recover the return value from syscall
+
+	pushl EAX(%esp)			# Recover the original system call #
+	pushl %eax
+	pushl %ebx
+	call do_bad_syscall_exit
 
 resume_kernelX:
 #endif
diff -X /home/apw/brief/lib/vdiff.excl -rupN reference/arch/i386/kernel/traps.c current/arch/i386/kernel/traps.c
--- reference/arch/i386/kernel/traps.c	2004-07-07 14:34:59.000000000 +0100
+++ current/arch/i386/kernel/traps.c	2004-07-07 14:56:37.000000000 +0100
@@ -874,6 +874,20 @@ asmlinkage void do_spurious_interrupt_bu
 #endif
 }
 
+#ifdef CONFIG_TRAP_BAD_SYSCALL_EXITS
+void do_bad_syscall_exit(struct pt_regs *regs, long syscall, long error_code)
+{
+	/*
+	 * Report the preempt count.  Then fix it so we can kill the
+	 * process and continue.  We _may_ get away with it.
+	 */
+	printk("Bad syscall exit - syscall %ld returned %ld preempt %08x\n",
+		syscall, error_code, preempt_count());
+	preempt_count() = 0;
+	die("Bad syscall exit - preempt non-zero", regs, syscall);
+}
+#endif
+
 /*
  *  'math_state_restore()' saves the current math information in the
  * old math state array, and gets the new ones from the current task