Re: [PATCH 1/3] kretprobes: ensure probe location is at function entry

[Naveen N. Rao]

On 2017/02/16 08:39AM, Masami Hiramatsu wrote:
> On Wed, 15 Feb 2017 23:47:52 +0530
> "Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > kretprobes can be registered by specifying an absolute address or by
> > specifying offset to a symbol. However, we need to ensure this falls at
> > function entry so as to be able to determine the return address.
> > 
> > Validate the same during kretprobe registration. By default, there
> > should not be any offset from a function entry, as determined through a
> > kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
> > way for architectures to override this.
> > 
> 
> Looks good to me.
> 
> Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thanks, Masami! I am cc'ing linux-arch in case any other architectures 
need to override the arch-specific helper with a custom offset.

- Naveen

> 
> Thanks!
> 
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
> > ---
> > powerpc64 ABIv2 will need to use the over-ride as we want to use the
> > local entry point which will be at an offset of 8 bytes from the
> > (global) entry point. I have a patch that I will post separately.
> > 
> > Thanks,
> > Naveen
> > 
> >  include/linux/kprobes.h |  1 +
> >  kernel/kprobes.c        | 13 +++++++++++++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> > index 8f6849084248..0c2489435117 100644
> > --- a/include/linux/kprobes.h
> > +++ b/include/linux/kprobes.h
> > @@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
> >  extern void show_registers(struct pt_regs *regs);
> >  extern void kprobes_inc_nmissed_count(struct kprobe *p);
> >  extern bool arch_within_kprobe_blacklist(unsigned long addr);
> > +extern bool arch_function_offset_within_entry(unsigned long offset);
> >  
> >  extern bool within_kprobe_blacklist(unsigned long addr);
> >  
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 43460104f119..72ecbf5a6312 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1834,12 +1834,25 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
> >  }
> >  NOKPROBE_SYMBOL(pre_handler_kretprobe);
> >  
> > +bool __weak arch_function_offset_within_entry(unsigned long offset)
> > +{
> > +	return !offset;
> > +}
> > +
> >  int register_kretprobe(struct kretprobe *rp)
> >  {
> >  	int ret = 0;
> >  	struct kretprobe_instance *inst;
> >  	int i;
> >  	void *addr;
> > +	unsigned long offset;
> > +
> > +	addr = kprobe_addr(&rp->kp);
> > +	if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, &offset))
> > +		return -EINVAL;
> > +
> > +	if (!arch_function_offset_within_entry(offset))
> > +		return -EINVAL;
> >  
> >  	if (kretprobe_blacklist_size) {
> >  		addr = kprobe_addr(&rp->kp);
> > -- 
> > 2.11.0
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>