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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2009-10-06 at 12:58 +0200, Joakim Tjernlund wrote:

> Here I don't care if err. insn will be 0 if it fails and the following
> if will be false

I'd rather you use get_user() so it does access_ok().

Else, you can probably manufacture some code that will make the kernel
access some MMIO register for example, which could be nasty.

At this point, you may as well also check the result even if indeed a
fault isn't going to matter. Just makes the code cleaner and avoids some
random janitor coming up with a patch later on :-)

Cheers,
Ben.

> > >        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;
> > > +         }
> 
> and here I go to bad_area