Re: [PATCH] uprobe: Add uprobe_pre/post_sstep_notifier to NOKPROBE_SYMBOL

[Pratyush Anand]

Hi Masami,

On 03/08/2016:12:45:24 AM, Masami Hiramatsu wrote:
> On Tue,  2 Aug 2016 12:14:06 +0530
> Pratyush Anand <panand@xxxxxxxxxx> wrote:
> 
> > uprobe_pre_sstep_notifier and uprobe_post_sstep_notifier are called from
> > debug exception handler, so blacklist them for kprobing.
> 
> Actually, these exception notifers are kicked only if the debug exception
> is not related to kprobes (at least on x86). In that case, we don't have
> to take care about that. Or, would you hit any problem on it?

Well, I have faced issue on ARM64. So, if I have a kprobe instrumented at these
functions and then if I hit a uprobe then kernel goes into an infinite loop of
"Unexpected kernel single-step exception at EL1".

On x86 I have not tested, but I see that all functions except
arch_uprobe_exception_notify() in the call stack of
uprobe_pre/post_sstep_notifier() are blacklisted for kprobe. So, I am unable to
understand that why arch_uprobe_exception_notify() and
uprobe_pre/post_sstep_notifier() are not blacklisted.

> 
> IOW, where do we have to prohibit kprobes are, the code path from where 
> right after the breakpoint (debug) exception is occurred, to where right
> before the kprobe is handled. After that, it should be safe.

Hummmm...My understanding was that if a function a() is not good to be kprobed
then we can not kprobe any function called by a() as well. Thanks for the
clarification. So, if I go with your definition then, something is still wrong on
ARM64 which is causing issue when I kprobe uprobe_pre/post_sstep_notifier().

~Pratyush

> 
> Thank you,
> 
> 
> > 
> > Signed-off-by: Pratyush Anand <panand@xxxxxxxxxx>
> > ---
> >  kernel/events/uprobes.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index b7a525ab2083..206e594cb65e 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/percpu-rwsem.h>
> >  #include <linux/task_work.h>
> >  #include <linux/shmem_fs.h>
> > +#include <linux/kprobes.h>
> >  
> >  #include <linux/uprobes.h>
> >  
> > @@ -1997,6 +1998,7 @@ int uprobe_pre_sstep_notifier(struct pt_regs *regs)
> >  	set_thread_flag(TIF_UPROBE);
> >  	return 1;
> >  }
> > +NOKPROBE_SYMBOL(uprobe_pre_sstep_notifier);
> >  
> >  /*
> >   * uprobe_post_sstep_notifier gets called in interrupt context as part of notifier
> > @@ -2014,6 +2016,7 @@ int uprobe_post_sstep_notifier(struct pt_regs *regs)
> >  	set_thread_flag(TIF_UPROBE);
> >  	return 1;
> >  }
> > +NOKPROBE_SYMBOL(uprobe_post_sstep_notifier);
> >  
> >  static struct notifier_block uprobe_exception_nb = {
> >  	.notifier_call		= arch_uprobe_exception_notify,
> > -- 
> > 2.5.5
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>