From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751218AbdJDToS (ORCPT ); Wed, 4 Oct 2017 15:44:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:58898 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751132AbdJDToR (ORCPT ); Wed, 4 Oct 2017 15:44:17 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 18FE8204EE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Wed, 4 Oct 2017 15:44:13 -0400 From: Steven Rostedt To: Masami Hiramatsu Cc: Jessica Yu , Petr Mladek , Ananth N Mavinakayanahalli , Anil S Keshavamurthy , "David S . Miller" , Ingo Molnar , Josh Poimboeuf , Joe Lawrence , Jiri Kosina , Miroslav Benes , live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/2] kprobes: improve error handling when arming/disarming kprobes Message-ID: <20171004154413.30daa238@gandalf.local.home> In-Reply-To: <20171004191414.8872-1-jeyu@kernel.org> References: <20171004191414.8872-1-jeyu@kernel.org> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; 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 Masami, Can you review these patches? -- Steve On Wed, 4 Oct 2017 21:14:12 +0200 Jessica Yu wrote: > Hi, > > This patchset attempts to improve error handling when arming or disarming > ftrace-based kprobes. The current behavior is to simply WARN when ftrace > (un-)registration fails, without propagating the error code. This can lead > to confusing situations where, for example, register_kprobe()/enable_kprobe() > would return 0 indicating success even if arming via ftrace had failed. In > this scenario we'd end up with a non-functioning kprobe even though kprobe > registration (or enablement) returned success. In this patchset, we take > errors from ftrace into account and propagate the error when we cannot arm > or disarm a kprobe. > > Below is an example that illustrates the problem using livepatch and > systemtap (which uses kprobes underneath). Both livepatch and kprobes use > ftrace ops with the IPMODIFY flag set, so registration at the same > function entry is limited to only one ftrace user. > > Before > ------ > # modprobe livepatch-sample # patches cmdline_proc_show, ftrace ops has IPMODIFY set > # stap -e 'probe kernel.function("cmdline_proc_show").call { printf ("cmdline_proc_show\n"); }' > > .. (nothing prints after reading /proc/cmdline) .. > > The systemtap handler doesn't execute due to a kprobe arming failure caused > by a ftrace IPMODIFY conflict with livepatch, and there isn't an obvious > indication of error from systemtap (because register_kprobe() returned > success) unless the user inspects dmesg. > > After > ----- > # modprobe livepatch-sample > # stap -e 'probe kernel.function("cmdline_proc_show").call { printf ("cmdline_proc_show\n"); }' > WARNING: probe kernel.function("cmdline_proc_show@/home/jeyu/work/linux-next/fs/proc/cmdline.c:6").call (address 0xffffffffa82fe910) registration error (rc -16) > > Although the systemtap handler doesn't execute (as it shouldn't), the > ftrace error is propagated and now systemtap prints a visible error message > stating that (kprobe) registration had failed (because register_kprobe() > returned an error), along with the propagated error code. > > This patchset was based on Petr Mladek's original patchset (patches 2 and 3) > back in 2015, which improved kprobes error handling, found here: > > https://lkml.org/lkml/2015/2/26/452 > > However, further work on this had been paused since then and the patches > were not upstreamed. > > This patchset has been lightly sanity-tested (on linux-next) with kprobes, > kretprobes, jprobes, and optimized kprobes. It passes the kprobes smoke > test, but more testing is greatly appreciated. > > Thanks, > > Jessica > > --- > Jessica Yu (2): > kprobes: propagate error from arm_kprobe_ftrace() > kprobes: propagate error from disarm_kprobe_ftrace() > > kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 112 insertions(+), 51 deletions(-) >