On Wed, 4 Oct 2017 15:44:13 -0400 Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > Masami, > > Can you review these patches? Yes, of course :) > > -- Steve > > > On Wed, 4 Oct 2017 21:14:12 +0200 > Jessica Yu <jeyu@xxxxxxxxxx> 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. Ah, right. It should be handled. > > > > 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. Would you have any test-case for this livepatch-kprobes combination? Thank you, > > > > 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(-) > > > -- Masami Hiramatsu <mhiramat@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html