From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755333AbdBOX3F (ORCPT ); Wed, 15 Feb 2017 18:29:05 -0500 Received: from mail.kernel.org ([198.145.29.136]:41184 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755180AbdBOX3C (ORCPT ); Wed, 15 Feb 2017 18:29:02 -0500 Date: Thu, 16 Feb 2017 08:28:51 +0900 From: Masami Hiramatsu To: "Naveen N. Rao" Cc: Ananth N Mavinakayanahalli , Ingo Molnar , linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo Subject: Re: [PATCH] kretprobes: reject registration if a symbol offset is specified Message-Id: <20170216082851.c98f4a3375b25071e686962d@kernel.org> In-Reply-To: <20170215175346.GE3169@naverao1-tp.localdomain> References: <57139b36e50748ad931d66658bed62b29280c8c2.1487060504.git.naveen.n.rao@linux.vnet.ibm.com> <20170214193221.a0f2f8799d0d303538fc329b@kernel.org> <20170215175346.GE3169@naverao1-tp.localdomain> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 15 Feb 2017 23:23:46 +0530 "Naveen N. Rao" wrote: > Hi Masami, > > On 2017/02/14 07:32PM, Masami Hiramatsu wrote: > > On Tue, 14 Feb 2017 14:01:18 +0530 > > "Naveen N. Rao" 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. Oops, it was so. That should be fixed correctly... > > 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. Yeah. > > > > > 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! > > 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 > > > --- > > > 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 > > > -- Masami Hiramatsu