x86: Unify printk strings in fault_32|64.c

PATCH Applied Harvey Harrison 3 participants 7 messages Jan 19, 2008

Subsystems (2)

x86 x86 MM

lore

Share & tools

lore

Harvey Harrison

author

2008-01-19 04:22 UTC #0 lore raw

PATCH 1/1 x86: Unify printk strings in fault_32|64.c

Adding the address of the faulting library missed removing a line ending from X86_32.

Also update the shorter printk format for X86_32 in fault_64.c to make it easier to se the remaining differences.

Signed-off-by Harvey Harrison <[email protected]>

---
Ingo, trivial printk update after Andi's patches.

 arch/x86/mm/fault_32.c |    2 +-
 arch/x86/mm/fault_64.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

arch/x86/mm/fault_32.c +1 -1

index 793e830..0bd2417 100644
@@ -589,7 +589,7 @@ bad_area_nosemaphore:
 		    printk_ratelimit()) {
 			printk(
 #ifdef CONFIG_X86_32
-			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else
 			"%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx",
 #endif

arch/x86/mm/fault_64.c +1 -1

index 9270a7d..9ac449e 100644
@@ -591,7 +591,7 @@ bad_area_nosemaphore:
 		    printk_ratelimit()) {
 			printk(
 #ifdef CONFIG_X86_32
-			"%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else
 			"%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx",
 #endif

Signature · 1 line

1.5.4.rc3.1118.gf6754c

Andi Kleen

2008-01-19 05:08 UTC #1 lore raw

Quoted 5 lines · On Saturday 19 January 2008 05:22:29 Harvey Harrison wrote:

Adding the address of the faulting library missed removing a line ending from X86_32.
Also update the shorter printk format for X86_32 in fault_64.c to make it easier to se the remaining differences.

Thanks. I think it was correct initially, but one of the merge steps with the changing git-x86 caused some hunks to be dropped and the patch never quite recovered from that.

-Andi

Harvey Harrison

author

2008-01-19 05:13 UTC #2 lore raw

Quoted 10 lines · Andi Kleen

Quoted 5 lines · On Saturday 19 January 2008 05:22:29 Harvey Harrison wrote:
Adding the address of the faulting library missed removing a line ending from X86_32.
Also update the shorter printk format for X86_32 in fault_64.c to make it easier to se the remaining differences.
Thanks. I think it was correct initially, but one of the merge steps with the changing git-x86 caused some hunks to be dropped and the patch never quite recovered from that.

No worries, hoping to get them unified this weekend, should make this easier going forward.

Harvey

Ingo Molnar

Applied

2008-01-19 08:13 UTC #3 lore raw

In reply to Harvey Harrison

arch/x86/mm/fault_64.c bad_area_nosemaphore: · line 595

Quoted 9 lines · Harvey Harrison

#ifdef CONFIG_X86_32

arch/x86/mm/fault_64.c +2 -2

-			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else
 
 #ifdef CONFIG_X86_32
-			"%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else

thanks, applied.

Ingo

Harvey Harrison

author

2008-01-19 09:04 UTC #4 lore raw

Quoted 7 lines · Ingo Molnar

Quoted 4 lines · Harvey Harrison
#ifdef CONFIG_X86_32
arch/x86/mm/fault_32.c +1 -1
-			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else

Quoted 8 lines

arch/x86/mm/fault_64.c bad_area_nosemaphore: · line 595
Quoted 4 lines
#ifdef CONFIG_X86_32
arch/x86/mm/fault_64.c +1 -1
-			"%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n",
+			"%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx",
 #else
thanks, applied.
Ingo

I'm just looking at some of the other printks in do+page_fault right now in the oops code.

What do you prefer:
"Unable to handle kernel NULL pointer dereference"
or
"BUG: unable to handle kernel NULL pointer dereference"

Other than that, I think for the most part, the shorter X86_64 strings are better.

This does change user-visible output however, but would be nice to have a similar format.

Harvey

Harvey Harrison

author

2008-01-19 10:17 UTC #5 lore raw

RFC PATCH 1/1 x86: Pull out oops printing helper from do_page_fault

In reply to Ingo Molnar

Pull out an show_fault_oops() helper out from the oops printing code in do_page_fault.

Try to make 32/64 bit oops traces have more similar format.

Signed-off-by Harvey Harrison <[email protected]>

---
Ingo, for discussion about how the oops traces should look, pull out a helper function. This changes the existing format.

 arch/x86/mm/fault_32.c |   53 ++++++++++++++++++++++++++---------------------
 arch/x86/mm/fault_64.c |   24 +++++++++++++++------
 2 files changed, 46 insertions(+), 31 deletions(-)

arch/x86/mm/fault_32.c +29 ~~-24~~

index 0bd2417..e15b80b 100644
@@ -396,6 +396,34 @@ static inline int vmalloc_fault(unsigned long address)
 #endif
 }
 
+static void show_fault_oops(struct pt_regs *regs, unsigned long error_code
+			    unsigned long address)
+{
+	if (!oops_may_print())
+		return;
+
+#ifdef CONFIG_X86_PAE
+	if (error_code & PF_INSTR) {
+		int level;
+		pte_t *pte = lookup_address(address, &level);
+
+		if (pte && pte_present(*pte) && !pte_exec(*pte))
+			printk(KERN_CRIT "kernel tried to execute "
+				"NX-protected page - exploit attempt? "
+				"(uid: %d)\n", current->uid);
+	}
+#endif
+	printk(KERN_ALERT "BUG: unable to handle kernel ");
+	if (address < PAGE_SIZE)
+		printk(KERN_CONT "NULL pointer dereference");
+	else
+		printk(KERN_CONT "paging request");
+
+	printk(KERN_CONT " at: %08lx IP: %08lx\n", address, regs->ip);
+
+	dump_pagetable(address);
+}
+
 int show_unhandled_signals = 1;
 
 /*

@@ -633,30 +661,7 @@ no_context:
 
 	bust_spinlocks(1);
 
-	if (oops_may_print()) {
-
-#ifdef CONFIG_X86_PAE
-		if (error_code & PF_INSTR) {
-			int level;
-			pte_t *pte = lookup_address(address, &level);
-
-			if (pte && pte_present(*pte) && !pte_exec(*pte))
-				printk(KERN_CRIT "kernel tried to execute "
-					"NX-protected page - exploit attempt? "
-					"(uid: %d)\n", current->uid);
-		}
-#endif
-		if (address < PAGE_SIZE)
-			printk(KERN_ALERT "BUG: unable to handle kernel NULL "
-					"pointer dereference");
-		else
-			printk(KERN_ALERT "BUG: unable to handle kernel paging"
-					" request");
-		printk(" at virtual address %08lx\n", address);
-		printk(KERN_ALERT "printing ip: %08lx ", regs->ip);
-
-		dump_pagetable(address);
-	}
+	show_fault_oops(regs, error_code, address);
 
 	tsk->thread.cr2 = address;
 	tsk->thread.trap_no = 14;

arch/x86/mm/fault_64.c +17 -7

index ccbb8e3..dce2893 100644
@@ -384,6 +384,21 @@ static int vmalloc_fault(unsigned long address)
 #endif
 }
 
+static void show_fault_oops(struct pt_regs *regs, unsigned long error_code
+			    unsigned long address)
+{
+	printk(KERN_ALERT "BUG: unable to handle kernel ");
+	if (address < PAGE_SIZE)
+		printk(KERN_CONT "NULL pointer dereference");
+	else
+		printk(KERN_CONT "paging request");
+
+	printk(KERN_CONT " at %016lx IP: \n", address);
+	printk_address(regs->ip, 1);
+
+	dump_pagetable(address);
+}
+
 int show_unhandled_signals = 1;
 
 /*

@@ -636,13 +651,8 @@ no_context:
 
 	flags = oops_begin();
 
-	if (address < PAGE_SIZE)
-		printk(KERN_ALERT "Unable to handle kernel NULL pointer dereference");
-	else
-		printk(KERN_ALERT "Unable to handle kernel paging request");
-	printk(" at %016lx RIP: \n" KERN_ALERT, address);
-	printk_address(regs->ip, 1);
-	dump_pagetable(address);
+	show_fault_oops(regs, error_code, address);
+
 	tsk->thread.cr2 = address;
 	tsk->thread.trap_no = 14;
 	tsk->thread.error_code = error_code;

Signature · 1 line

1.5.4.rc3.1118.gf6754c

Harvey Harrison

author

2008-01-19 10:26 UTC #6 lore raw

Re: [RFC PATCHv2] x86: Pull out oops printing helper from do_page_fault

Pull out an show_fault_oops() helper out from the oops printing code in do_page_fault.

Try to make 32/64 bit oops traces have more similar format.

Signed-off-by Harvey Harrison <[email protected]>

---
Clearly that last patch was for discussion only, he's one that actually compiles (missing comma on args in show_fault_oops)

 arch/x86/mm/fault_32.c |   53 ++++++++++++++++++++++++++---------------------
 arch/x86/mm/fault_64.c |   24 +++++++++++++++------
 2 files changed, 46 insertions(+), 31 deletions(-)

arch/x86/mm/fault_32.c +29 ~~-24~~

index 0bd2417..83b4c1a 100644
@@ -396,6 +396,34 @@ static inline int vmalloc_fault(unsigned long address)
 #endif
 }
 
+static void show_fault_oops(struct pt_regs *regs, unsigned long error_code,
+			    unsigned long address)
+{
+	if (!oops_may_print())
+		return;
+
+#ifdef CONFIG_X86_PAE
+	if (error_code & PF_INSTR) {
+		int level;
+		pte_t *pte = lookup_address(address, &level);
+
+		if (pte && pte_present(*pte) && !pte_exec(*pte))
+			printk(KERN_CRIT "kernel tried to execute "
+				"NX-protected page - exploit attempt? "
+				"(uid: %d)\n", current->uid);
+	}
+#endif
+	printk(KERN_ALERT "BUG: unable to handle kernel ");
+	if (address < PAGE_SIZE)
+		printk(KERN_CONT "NULL pointer dereference");
+	else
+		printk(KERN_CONT "paging request");
+
+	printk(KERN_CONT " at: %08lx IP: %08lx\n", address, regs->ip);
+
+	dump_pagetable(address);
+}
+
 int show_unhandled_signals = 1;
 
 /*

@@ -633,30 +661,7 @@ no_context:
 
 	bust_spinlocks(1);
 
-	if (oops_may_print()) {
-
-#ifdef CONFIG_X86_PAE
-		if (error_code & PF_INSTR) {
-			int level;
-			pte_t *pte = lookup_address(address, &level);
-
-			if (pte && pte_present(*pte) && !pte_exec(*pte))
-				printk(KERN_CRIT "kernel tried to execute "
-					"NX-protected page - exploit attempt? "
-					"(uid: %d)\n", current->uid);
-		}
-#endif
-		if (address < PAGE_SIZE)
-			printk(KERN_ALERT "BUG: unable to handle kernel NULL "
-					"pointer dereference");
-		else
-			printk(KERN_ALERT "BUG: unable to handle kernel paging"
-					" request");
-		printk(" at virtual address %08lx\n", address);
-		printk(KERN_ALERT "printing ip: %08lx ", regs->ip);
-
-		dump_pagetable(address);
-	}
+	show_fault_oops(regs, error_code, address);
 
 	tsk->thread.cr2 = address;
 	tsk->thread.trap_no = 14;

arch/x86/mm/fault_64.c +17 -7

index ccbb8e3..34f50f6 100644
@@ -384,6 +384,21 @@ static int vmalloc_fault(unsigned long address)
 #endif
 }
 
+static void show_fault_oops(struct pt_regs *regs, unsigned long error_code,
+			    unsigned long address)
+{
+	printk(KERN_ALERT "BUG: unable to handle kernel ");
+	if (address < PAGE_SIZE)
+		printk(KERN_CONT "NULL pointer dereference");
+	else
+		printk(KERN_CONT "paging request");
+
+	printk(KERN_CONT " at %016lx IP: \n", address);
+	printk_address(regs->ip, 1);
+
+	dump_pagetable(address);
+}
+
 int show_unhandled_signals = 1;
 
 /*

@@ -636,13 +651,8 @@ no_context:
 
 	flags = oops_begin();
 
-	if (address < PAGE_SIZE)
-		printk(KERN_ALERT "Unable to handle kernel NULL pointer dereference");
-	else
-		printk(KERN_ALERT "Unable to handle kernel paging request");
-	printk(" at %016lx RIP: \n" KERN_ALERT, address);
-	printk_address(regs->ip, 1);
-	dump_pagetable(address);
+	show_fault_oops(regs, error_code, address);
+
 	tsk->thread.cr2 = address;
 	tsk->thread.trap_no = 14;
 	tsk->thread.error_code = error_code;

Signature · 1 line

1.5.4.rc3.1118.gf6754c