Re: [PATCH] x86 page fault handler not interrupt safe

[Brian Gerst]

Linus Torvalds wrote:
> 
> On Tue, 8 May 2001, Alan Cox wrote:
> >
> > I dont see where the alternative patch ensures the user didnt flip the
> > direction flag for one
> 
> Yeah.
> 
> We might as well just make it "eflags & IF", none of the other flags
> should matter (or we explicitly want them cleared).
> 
>                 Linus

Here is an updated patch.  After reading over the Intel docs, and some
testing on my Athlon, I found that %cr2 is not set on alignment check
faults.  I replaced it with the address of the faulting instruction.  It
may work on an Intel but is undocumented.  The eip makes more sense
anyways.

-- 

                                                Brian Gerst
diff -urN linux-2.4.5-pre1/arch/i386/kernel/traps.c linux/arch/i386/kernel/traps.c
--- linux-2.4.5-pre1/arch/i386/kernel/traps.c   Mon Mar 19 21:23:40 2001
+++ linux/arch/i386/kernel/traps.c      Wed May  9 17:51:58 2001
@@ -225,15 +225,6 @@
                die(str, regs, err);
 }
 
-static inline unsigned long get_cr2(void)
-{
-       unsigned long address;
-
-       /* get the address */
-       __asm__("movl %%cr2,%0":"=r" (address));
-       return address;
-}
-
 static void inline do_trap(int trapnr, int signr, char *str, int vm86,
                           struct pt_regs * regs, long error_code, siginfo_t *info)
 {
@@ -314,7 +305,7 @@
 DO_ERROR(10, SIGSEGV, "invalid TSS", invalid_TSS)
 DO_ERROR(11, SIGBUS,  "segment not present", segment_not_present)
 DO_ERROR(12, SIGBUS,  "stack segment", stack_segment)
-DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, get_cr2())
+DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, regs->eip)
 
 asmlinkage void do_general_protection(struct pt_regs * regs, long error_code)
 {
@@ -973,7 +964,7 @@
        set_trap_gate(11,&segment_not_present);
        set_trap_gate(12,&stack_segment);
        set_trap_gate(13,&general_protection);
-       set_trap_gate(14,&page_fault);
+       set_intr_gate(14,&page_fault);
        set_trap_gate(15,&spurious_interrupt_bug);
        set_trap_gate(16,&coprocessor_error);
        set_trap_gate(17,&alignment_check);
diff -urN linux-2.4.5-pre1/arch/i386/mm/fault.c linux/arch/i386/mm/fault.c
--- linux-2.4.5-pre1/arch/i386/mm/fault.c       Wed May  2 09:24:09 2001
+++ linux/arch/i386/mm/fault.c  Wed May  9 17:18:17 2001
@@ -98,6 +98,9 @@
  * and the problem, and then passes it off to one of the appropriate
  * routines.
  *
+ * This is called with interrupts off, to protect %cr2 from being
+ * overwritten by an interrupt handler that faults.
+ *
  * error_code:
  *     bit 0 == 0 means no page found, 1 means protection fault
  *     bit 1 == 0 means read, 1 means write
@@ -116,6 +119,10 @@
 
        /* get the address */
        __asm__("movl %%cr2,%0":"=r" (address));
+
+       /* Reenable interrupts, but don't trust any other flags */
+       if (regs->eflags & X86_EFLAGS_IF)
+               sti();
 
        tsk = current;