[PATCH 0/6] PowerPc 8xx TLB/MMU fixes

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh at kernel.crashing.org> wrote on 06/10/2009 03:52:15:
>
> \
> > So how does this look? Does it change anything?
> > It should as the previous way was way off :(
> >
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index c33c6de..08a392f 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -153,7 +153,7 @@ int __kprobes do_page_fault(struct pt_regs *regs,
> unsigned long address,
> >  #ifdef DEBUG_DCBX
> >        const char *istr = NULL;
> >
> > -      insn = *((unsigned long *)regs->nip);
> > +      __get_user(insn, (unsigned long __user *)regs->nip);
>
> No, use get_user() not __get_user() or if you use the later, also use
> access_ok(), and test the result in case it errors (if it does, you
> probably want to just goto bad access and SEGV).

OK, lets see what this gives us:

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index c33c6de..1bf91d3 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -153,7 +153,8 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
 #ifdef DEBUG_DCBX
 		const char *istr = NULL;

-		insn = *((unsigned long *)regs->nip);
+		insn = 0;
+		__get_user(insn, (unsigned long __user *)regs->nip);
 		if (((insn >> (31-5)) & 0x3f) == 31) {
 			if (((insn >> 1) & 0x3ff) == 1014) /* dcbz ? 0x3f6 */
 				istr = "dcbz";
@@ -171,27 +172,32 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
 				dar = regs->gpr[rb];
 				if (ra)
 					dar += regs->gpr[ra];
-				if (dar != address && address != 0x00f0 && trap == 0x300)
+				if (dar != address && trap == 0x300)
 					printk(KERN_CRIT "%s: address:%lx, dar:%lx!\n", istr, address, dar);
 				if (!strcmp(istr, "dcbst") && is_write) {
 					printk(KERN_CRIT "dcbst R%ld,R%ld = %lx as a store, fixing!\n",
 					       ra, rb, dar);
 					is_write = 0;
 				}
-
+#if 0
 				if (trap == 0x300 && address != dar) {
 					__asm__ ("mtdar %0" : : "r" (dar));
 					return 0;
 				}
+#endif
 			}
 		}
 #endif
 		if (address == 0x00f0 && trap == 0x300) {
-			pte_t *ptep;
+			//pte_t *ptep;

 			/* This is from a dcbX or icbi insn gone bad, these
 			 * insn do not set DAR so we have to do it here instead */
-			insn = *((unsigned long *)regs->nip);
+			if (get_user(insn, (unsigned long __user *)regs->nip)) {
+				printk(KERN_CRIT "get_user failed, NIP:%lx\n",
+				       regs->nip);
+				goto bad_area_nosemaphore;
+			}

 			ra = (insn >> (31-15)) & 0x1f; /* Reg RA */
 			rb = (insn >> (31-20)) & 0x1f; /* Reg RB */
@@ -206,7 +212,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
 			       trap, address, dar, error_code, istr);
 #endif
 			address = dar;
-#if 1
+#if 0
 			if (is_write && get_pteptr(mm, dar, &ptep, NULL)) {
 				pte_t my_pte = *ptep;

@@ -216,7 +222,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
 				}
 			}
 #else
-			return 0;
+			//return 0;
 #endif
 		}
 	}