Hi Masami, On 2017/02/14 07:32PM, Masami Hiramatsu wrote: > On Tue, 14 Feb 2017 14:01:18 +0530 > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote: > > > Users shouldn't be able to specify an offset with kretprobes, as we always > > want to probe at function entry. Otherwise, we won't be able to capture > > the proper return address resulting in the kretprobe never firing. > > > > Nack, this should be checked by using kallsyms, since the > many non-exported kernel functions have same name. > Actually perf-probe is trying to put any probes(including return > probe) by using relative address from text-start symbol (_stext > or _text). In this case, kretprobe also can be set by _text+OFFSET.
Interesting. In my tests, 'perf probe' always chose the function name for return probes on both x86 and powerpc. So, looking into it further, I found commit 25dd9171f51c ("perf probe: Fix probing kretprobes") which changed perf probe behavior due to how kprobe_events behaved. And kprobe_events has always dis-allowed use of offset with kprobe_events. But, I agree -- we should allow use of offset with kretprobes. Otherwise, we won't be able to probe functions that have the same name. > > So please rewrite this by using kallsyms_lookup_size_offset() > which tells you the address is actually on the beginning of > function or not. Sure. This makes use of offsets and/or absolute addresses with kretprobes safe. Patches on the way... Thanks! - Naveen > > Thank you, > > > With samples/kprobes/kretprobe_example.c including an offset: > > my_kretprobe.kp.offset = 40; > > > > Before this patch, the probe gets planted but never fires. > > > > After this patch: > > $ sudo insmod samples/kprobes/kretprobe_example.ko > > [sudo] password for naveen: > > insmod: ERROR: could not insert module > > samples/kprobes/kretprobe_example.ko: Operation not permitted > > > > And dmesg: > > [48253.757629] register_kretprobe failed, returned -22 > > > > Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com> > > --- > > kernel/kprobes.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > > index 60a702a05684..83ad7e440417 100644 > > --- a/kernel/kprobes.c > > +++ b/kernel/kprobes.c > > @@ -1847,6 +1847,9 @@ int register_kretprobe(struct kretprobe *rp) > > int i; > > void *addr; > > > > + if (rp->kp.offset) > > + return -EINVAL; > > + > > if (kretprobe_blacklist_size) { > > addr = kprobe_addr(&rp->kp); > > if (IS_ERR(addr)) > > -- > > 2.11.0 > > > > > -- > Masami Hiramatsu <mhira...@kernel.org> >