[PATCH] i386: make BUG() expansion look like instruction

[PATCH] i386: make BUG() expansion look like instruction

[Jan Beulich]

This gets i386's handle_BUG() in sync with x86-64's, namely introducing
a 'struct bug_frame' and arranging for the code generated by the BUG()
macro to represent an instruction, so that disassembling code
containing such constructs doesn't produce garbage anymore. The
resulting increase of code size is marginal (1 byte per instance).

Unfortunately this results in the new uses of probe_kernel_address()
getting removed again (they were incomplete anyway) - the question is
whether explicit calls to {inc,dec}_preempt_count should be added to
this code (and then also to x86-64's).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.19-rc5/arch/i386/kernel/traps.c	2006-11-08 09:21:37.000000000 +0100
+++ 2.6.19-rc5-i386-bug/arch/i386/kernel/traps.c	2006-11-08 15:17:11.000000000 +0100
@@ -406,32 +406,35 @@ void show_registers(struct pt_regs *regs
 
 static void handle_BUG(struct pt_regs *regs)
 {
-	unsigned long eip = regs->eip;
-	unsigned short ud2;
+	struct bug_frame f;
 
-	if (eip < PAGE_OFFSET)
+	if (regs->eip < PAGE_OFFSET)
 		return;
-	if (probe_kernel_address((unsigned short __user *)eip, ud2))
+	if (__copy_from_user(&f, (const void __user *)regs->eip, sizeof(f)))
 		return;
-	if (ud2 != 0x0b0f)
+	if (f.ud2[0] != 0x0f || f.ud2[1] != 0x0b)
 		return;
 
 	printk(KERN_EMERG "------------[ cut here ]------------\n");
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 	do {
-		unsigned short line;
-		char *file;
-		char c;
-
-		if (probe_kernel_address((unsigned short __user *)(eip + 2),
-					line))
-			break;
-		if (__get_user(file, (char * __user *)(eip + 4)) ||
-		    (unsigned long)file < PAGE_OFFSET || __get_user(c, file))
-			file = "<bad filename>";
+		long len;
+		const char *prefix = "";
 
-		printk(KERN_EMERG "kernel BUG at %s:%d!\n", file, line);
+		if (f.filename < PAGE_OFFSET)
+			len = -1;
+		else
+			len = __strnlen_user((const char __user *)f.filename,
+					     PATH_MAX) - 1;
+		if (len < 0 || len >= PATH_MAX)
+			f.filename = (unsigned long)"<bad filename>";
+		else if (len > 50) {
+			f.filename += len - 50;
+			prefix = "...";
+		}
+		printk(KERN_EMERG "kernel BUG at %s%.50s:%d!\n",
+		       prefix, (char *)f.filename, f.line);
 		return;
 	} while (0);
 #endif
--- linux-2.6.19-rc5/arch/i386/lib/usercopy.c	2006-11-08 09:21:37.000000000 +0100
+++ 2.6.19-rc5-i386-bug/arch/i386/lib/usercopy.c	2006-11-08 15:06:51.000000000 +0100
@@ -223,6 +223,36 @@ long strnlen_user(const char __user *s, 
 }
 EXPORT_SYMBOL(strnlen_user);
 
+long __strnlen_user(const char __user *s, long n)
+{
+	unsigned long res;
+
+	__asm__ __volatile__(
+		"	testl %0, %0\n"
+		"	jz 3f\n"
+		"	movl %0,%%ecx\n"
+		"0:	repne; scasb\n"
+		"	setne %%al\n"
+		"	subl %%ecx,%0\n"
+		"	addl %0,%%eax\n"
+		"1:\n"
+		".section .fixup,\"ax\"\n"
+		"2:	xorl %%eax,%%eax\n"
+		"	jmp 1b\n"
+		"3:	movb $1,%%al\n"
+		"	jmp 1b\n"
+		".previous\n"
+		".section __ex_table,\"a\"\n"
+		"	.align 4\n"
+		"	.long 0b,2b\n"
+		".previous"
+		:"=r" (n), "=D" (s), "=a" (res)
+		:"0" (n), "1" (s), "2" (0)
+		:"ecx", "cc");
+	return res;
+}
+EXPORT_SYMBOL(__strnlen_user);
+
 #ifdef CONFIG_X86_INTEL_USERCOPY
 static unsigned long
 __copy_user_intel(void __user *to, const void *from, unsigned long size)
--- linux-2.6.19-rc5/include/asm-i386/bug.h	2006-09-20 05:42:06.000000000 +0200
+++ 2.6.19-rc5-i386-bug/include/asm-i386/bug.h	2006-11-08 15:16:20.000000000 +0100
@@ -7,14 +7,21 @@
  * The offending file and line are encoded after the "officially
  * undefined" opcode for parsing in the trap handler.
  */
+struct bug_frame {
+	unsigned char ud2[2];
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+	unsigned char ljmp;
+	unsigned long filename;
+	unsigned short line;
+#endif
+} __attribute__((packed));
 
 #ifdef CONFIG_BUG
 #define HAVE_ARCH_BUG
 #ifdef CONFIG_DEBUG_BUGVERBOSE
-#define BUG()				\
- __asm__ __volatile__(	"ud2\n"		\
-			"\t.word %c0\n"	\
-			"\t.long %c1\n"	\
+#define BUG()					\
+ __asm__ __volatile__(	"ud2\n\t"		\
+			"ljmp $%c0,$%c1"	\
 			 : : "i" (__LINE__), "i" (__FILE__))
 #else
 #define BUG() __asm__ __volatile__("ud2\n")
--- linux-2.6.19-rc5/include/asm-i386/uaccess.h	2006-11-08 09:22:26.000000000 +0100
+++ 2.6.19-rc5-i386-bug/include/asm-i386/uaccess.h	2006-11-08 14:52:38.000000000 +0100
@@ -572,6 +572,7 @@ long __must_check __strncpy_from_user(ch
 #define strlen_user(str) strnlen_user(str, ~0UL >> 1)
 
 long strnlen_user(const char __user *str, long n);
+long __strnlen_user(const char __user *str, long n);
 unsigned long __must_check clear_user(void __user *mem, unsigned long len);
 unsigned long __must_check __clear_user(void __user *mem, unsigned long len);
 

Re: [PATCH] i386: make BUG() expansion look like instruction

[Hugh Dickins]

On Wed, 8 Nov 2006, Jan Beulich wrote:
> This gets i386's handle_BUG() in sync with x86-64's, namely introducing
> a 'struct bug_frame' and arranging for the code generated by the BUG()
> macro to represent an instruction, so that disassembling code
> containing such constructs doesn't produce garbage anymore. The
> resulting increase of code size is marginal (1 byte per instance).

I thought we were heading towards merging Jeremy's generic-bug-*
(with file and line in another section), in -mm for a while now?

Hugh

Re: [PATCH] i386: make BUG() expansion look like instruction

[Jeremy Fitzhardinge]

Hugh Dickins wrote:
> On Wed, 8 Nov 2006, Jan Beulich wrote:
>   
>> This gets i386's handle_BUG() in sync with x86-64's, namely introducing
>> a 'struct bug_frame' and arranging for the code generated by the BUG()
>> macro to represent an instruction, so that disassembling code
>> containing such constructs doesn't produce garbage anymore. The
>> resulting increase of code size is marginal (1 byte per instance).
>>     
>
> I thought we were heading towards merging Jeremy's generic-bug-*
> (with file and line in another section), in -mm for a while now?
>   

That's certainly my impression.  Andrew already NAKed my original patch 
which did something similar to Jan's, in favour of making the powerpc 
BUG handling code generic.  I'm hoping the generic bug can be put in 
mainline fairly soon, and then encourage other arch maintainers to use it.

I think there's still some question in the air over powerpc's use of the 
generic bug stuff, but as far as I know Andrew is the only person to 
have a problem with it.

    J