Skip to contentSkip to conversation

add TRAP_BAD_SYSCALL_EXITS config for i386

RFC PATCH In review Andy Whitcroft 2 participants 3 messages Jul 2, 2004 – Jul 7, 2004

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 add TRAP_BAD_SYSCALL_EXITS config for i386

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 $

---
 Kconfig |    7 +++++++
 1 files changed, 7 insertions(+)
arch/i386/Kconfig +7 -0
diff -upN reference/arch/i386/Kconfig current/arch/i386/Kconfig
@@ -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"
#1 lore raw
Quoted 7 lines · 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?

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.

#2 lore raw
Quoted 5 lines

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 $

---

arch/i386/kernel/entry.S +7 -1
diff -X /home/apw/brief/lib/vdiff.excl -rupN reference/arch/i386/kernel/entry.S current/arch/i386/kernel/entry.S
@@ -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
arch/i386/kernel/traps.c +14 -0
diff -X /home/apw/brief/lib/vdiff.excl -rupN reference/arch/i386/kernel/traps.c current/arch/i386/kernel/traps.c
@@ -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