Was this patch ever picked up in the tip tree? (Maybe I'm not looking
in the right branch?)
Thanks,
-- Joe
On 01/19/2018 02:06 AM, Masami Hiramatsu wrote:
> Hi Ingo,
>
> Could you pick this to tip tree?
>
> Thank you,
>
> On Wed, 10 Jan 2018 00:51:22 +0100
> 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.
>>
>> 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, and optimized kprobes. It passes the kprobes smoke test, but
>> more testing is greatly appreciated.
>>
>> Changes from v4:
>> - Switch from WARN() to pr_debug() in arm_kprobe_ftrace() so the stack
>> dumps don't pollute dmesg, as IPMODIFY conflicts can occur in normal usage
>> - Added Masami's ack to the first patch
>>
>> Changes from v3:
>> - Have (dis)arm_kprobe_ftrace() return -ENODEV instead of 0 in case of
>> !CONFIG_KPROBES_ON_FTRACE
>> - Add total count of all probes tried in (dis)arm_all_kprobes()
>>
>> Changes from v2:
>> - Add missing synchronize rcu in register_aggr_kprobe()
>> - s/kprobes/probes/ on error message in (dis)arm_all_kprobes()
>>
>> Changes from v1:
>> - Don't arm the kprobe before adding it to the kprobe table, otherwise
>> we'll temporarily see a stray breakpoint.
>> - Remove kprobe from the kprobe_table and call synchronize_sched() if
>> arming during register_kprobe() fails.
>> - add Masami's ack on the 2nd patch (unchanged from v1)
>>
>> ---
>> Jessica Yu (2):
>> kprobes: propagate error from arm_kprobe_ftrace()
>> kprobes: propagate error from disarm_kprobe_ftrace()
>>
>> kernel/kprobes.c | 178 +++++++++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 128 insertions(+), 50 deletions(-)
>>
>> --
>> 2.13.6
>>
>
>
--
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
