Re: [PATCH] powerpc/kprobes: Fix trap address when trap happened in real mode

[Christophe Leroy]

Le 16/02/2020 Ã 13:34, Masami Hiramatsu a ÃcritÂ:
> On Sat, 15 Feb 2020 11:28:49 +0100
> Christophe Leroy <christophe.leroy@xxxxxx> wrote:
>
> > Hi,
> >
> > Le 14/02/2020 Ã 14:54, Masami Hiramatsu a ÃcritÂ:
> > > Hi,
> > >
> > > On Fri, 14 Feb 2020 12:47:49 +0000 (UTC)
> > > Christophe Leroy <christophe.leroy@xxxxxx> wrote:
> > >
> > > > When a program check exception happens while MMU translation is
> > > > disabled, following Oops happens in kprobe_handler() in the following
> > > > test:
> > > >
> > > > 		} else if (*addr != BREAKPOINT_INSTRUCTION) {
> > >
> > > Thanks for the report and patch. I'm not so sure about powerpc implementation
> > > but at where the MMU translation is disabled, can the handler work correctly?
> > > (And where did you put the probe on?)
> > >
> > > Your fix may fix this Oops, but if the handler needs special care, it is an
> > > option to blacklist such place (if possible).
> >
> > I guess that's another story. Here we are not talking about a place
> > where kprobe has been illegitimately activated, but a place where there
> > is a valid trap, which generated a valid 'program check exception'. And
> > kprobe was off at that time.
>
> Ah, I got it. It is not a kprobe breakpoint, but to check that correctly,
> it has to know the address where the breakpoint happens. OK.
>
> > As any 'program check exception' due to a trap (ie a BUG_ON, a WARN_ON,
> > a debugger breakpoint, a perf breakpoint, etc...) calls
> > kprobe_handler(), kprobe_handler() must be prepared to handle the case
> > where the MMU translation is disabled, even if probes are not supposed
> > to be set for functions running with MMU translation disabled.
>
> Can't we check the MMU is disabled there (as same as checking the exception
> happened in user space or not)?

What do you mean by 'there' ? At the entry of kprobe_handler() ?

That's what my patch does, it checks whether MMU is disabled or not. If 
it is, it converts the address to a virtual address.

Do you mean kprobe_handler() should bail out early as it does when the 
trap happens in user mode ? Of course we can do that, I don't know 
enough about kprobe to know if kprobe_handler() should manage events 
that happened in real-mode or just ignore them. But I tested adding an 
event on a function that runs in real-mode, and it (now) works.

So, what should we do really ?

Christophe