[patch] i386: fix recursive fault in page-fault handler

[patch] i386: fix recursive fault in page-fault handler

[Chuck Ebbert]

Krzysztof Halasa reported recursive faults in do_page_fault()
causing a stream of partial oops messages on the console. Fix
by adding a fixup for that code.

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

---

x86_64 looks like it has the same problem.

--- 2.6.18-rc1-32.orig/arch/i386/mm/fault.c
+++ 2.6.18-rc1-32/arch/i386/mm/fault.c
@@ -585,9 +585,20 @@ no_context:
 		printk(KERN_ALERT "*pte = %08lx\n", page);
 	}
 #endif
-	tsk->thread.cr2 = address;
-	tsk->thread.trap_no = 14;
-	tsk->thread.error_code = error_code;
+	asm (	"# set task data without causing another oops\n"
+		"1:\t"
+		"movl %3,%0\n\t"
+		"movl $14,%1\n\t"
+		"movl %4,%2\n"
+		"2:\n"
+		".section __ex_table,\"a\"\n\t"
+		".align 4\n\t"
+		".long 1b,2b\n"
+		".previous"
+		: "=m" (tsk->thread.cr2), "=m" (tsk->thread.trap_no),
+		  "=m" (tsk->thread.error_code)
+		: "r" (address), "r" (error_code)
+	);
 	die("Oops", regs, error_code);
 	bust_spinlocks(0);
 	do_exit(SIGKILL);
-- 
And did you exchange a walk-on part in the war for a lead role in a cage?
        --Roger Waters

Re: [patch] i386: fix recursive fault in page-fault handler

[Linus Torvalds]

On Mon, 17 Jul 2006, Chuck Ebbert wrote:
>
> Krzysztof Halasa reported recursive faults in do_page_fault()
> causing a stream of partial oops messages on the console. Fix
> by adding a fixup for that code.

This patch is really too ugly to live. Does it even work? If 'tsk' is 
broken, I'd expect the die() to oops anyway - it does

	if (notify_die(DIE_OOPS, str, regs, err,
                       current->thread.trap_no, SIGSEG...

anyway (where that "current->thread.trap_no" gets dereferenced).

		Linus

Re: [patch] i386: fix recursive fault in page-fault handler

[Andi Kleen]

On Monday 17 July 2006 19:19, Chuck Ebbert wrote:
> Krzysztof Halasa reported recursive faults in do_page_fault()
> causing a stream of partial oops messages on the console. Fix
> by adding a fixup for that code.

Please just use __put_user, no need to do it in full inline assembly

-Andi

Re: [patch] i386: fix recursive fault in page-fault handler

[Chuck Ebbert]

In-Reply-To: <Pine.LNX.4.64.0607171107390.15611@evo.osdl.org>

On Mon, 17 Jul 2006 11:08:26 -0700 (PDT), Linus Torvalds wrote:
> 
> On Mon, 17 Jul 2006, Chuck Ebbert wrote:
> >
> > Krzysztof Halasa reported recursive faults in do_page_fault()
> > causing a stream of partial oops messages on the console. Fix
> > by adding a fixup for that code.
> 
> This patch is really too ugly to live.

I was afraid to use __put_user, but I guess it's OK?

--- 2.6.18-rc1-32.orig/arch/i386/mm/fault.c
+++ 2.6.18-rc1-32/arch/i386/mm/fault.c
@@ -585,9 +585,10 @@ no_context:
 		printk(KERN_ALERT "*pte = %08lx\n", page);
 	}
 #endif
-	tsk->thread.cr2 = address;
-	tsk->thread.trap_no = 14;
-	tsk->thread.error_code = error_code;
+	/* avoid possible fault here if tsk is garbage */
+	__put_user(address, &tsk->thread.cr2);
+	__put_user(14, &tsk->thread.trap_no);
+	__put_user(error_code, &tsk->thread.error_code);
 	die("Oops", regs, error_code);
 	bust_spinlocks(0);
 	do_exit(SIGKILL);

> Does it even work? If 'tsk' is 
> broken, I'd expect the die() to oops anyway - it does
> 
>       if (notify_die(DIE_OOPS, str, regs, err,
>                        current->thread.trap_no, SIGSEG...
> 
> anyway (where that "current->thread.trap_no" gets dereferenced).

This should at least stop the endless faults because recursive faulting
in die() is handled properly.  Right now the original error message
(incomplete but still possibly useful) scrolls away.

I was going to fix handling of bad task pointer in die() and
show_registers() after I got feedback from the first patch.

-- 
Chuck