From patchwork Wed Oct 31 15:25:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aleksa Sarai X-Patchwork-Id: 10662847 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D56BE17DB for ; Wed, 31 Oct 2018 15:26:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C26BE29341 for ; Wed, 31 Oct 2018 15:26:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B46A6293BD; Wed, 31 Oct 2018 15:26:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CA04629341 for ; Wed, 31 Oct 2018 15:26:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728839AbeKAAYr (ORCPT ); Wed, 31 Oct 2018 20:24:47 -0400 Received: from mx2.mailbox.org ([80.241.60.215]:35358 "EHLO mx2.mailbox.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728698AbeKAAYr (ORCPT ); Wed, 31 Oct 2018 20:24:47 -0400 Received: from smtp1.mailbox.org (unknown [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mx2.mailbox.org (Postfix) with ESMTPS id EF8A7A120F; Wed, 31 Oct 2018 16:26:15 +0100 (CET) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter05.heinlein-hosting.de (spamfilter05.heinlein-hosting.de [80.241.56.123]) (amavisd-new, port 10030) with ESMTP id eL9jtq5T92kY; Wed, 31 Oct 2018 16:26:09 +0100 (CET) From: Aleksa Sarai To: "Naveen N. Rao" , Anil S Keshavamurthy , "David S. Miller" , Masami Hiramatsu , Jonathan Corbet , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Steven Rostedt , Shuah Khan , Alexei Starovoitov , Daniel Borkmann Cc: Aleksa Sarai , Brendan Gregg , Christian Brauner , Aleksa Sarai , netdev@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v2 1/2] kretprobe: produce sane stack traces Date: Thu, 1 Nov 2018 02:25:42 +1100 Message-Id: <20181031152543.12138-2-cyphar@cyphar.com> In-Reply-To: <20181031152543.12138-1-cyphar@cyphar.com> References: <20181031152543.12138-1-cyphar@cyphar.com> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Historically, kretprobe has always produced unusable stack traces (kretprobe_trampoline is the only entry in most cases, because of the funky stack pointer overwriting). This has caused quite a few annoyances when using tracing to debug problems[1] -- since return values are only available with kretprobes but stack traces were only usable for kprobes, users had to probe both and then manually associate them. With the advent of bpf_trace, users would have been able to do this association in bpf, but this was less than ideal (because bpf_get_stackid would still produce rubbish and programs that didn't know better would get silly results). The main usecase for stack traces (at least with bpf_trace) is for DTrace-style aggregation on stack traces (both entry and exit). Therefore we cannot simply correct the stack trace on exit -- we must stash away the stack trace and return the entry stack trace when it is requested. [1]: https://github.com/iovisor/bpftrace/issues/101 Cc: Brendan Gregg Cc: Christian Brauner Signed-off-by: Aleksa Sarai --- Documentation/kprobes.txt | 6 +- include/linux/kprobes.h | 15 +++ kernel/events/callchain.c | 8 +- kernel/kprobes.c | 101 +++++++++++++++++- kernel/trace/trace.c | 11 +- .../test.d/kprobe/kretprobe_stacktrace.tc | 25 +++++ 6 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt index 10f4499e677c..1965585848f4 100644 --- a/Documentation/kprobes.txt +++ b/Documentation/kprobes.txt @@ -597,7 +597,11 @@ address with the trampoline's address, stack backtraces and calls to __builtin_return_address() will typically yield the trampoline's address instead of the real return address for kretprobed functions. (As far as we can tell, __builtin_return_address() is used only -for instrumentation and error reporting.) +for instrumentation and error reporting.) However, since return probes +are used extensively in tracing (where stack backtraces are useful), +return probes will stash away the stack backtrace during function entry +so that return probe handlers can use the entry backtrace instead of +having a trace with just kretprobe_trampoline. If the number of times a function is called does not match the number of times it returns, registering a return probe on that function may diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index e909413e4e38..b2d6e8b74731 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -40,6 +40,8 @@ #include #include #include +#include +#include #include #ifdef CONFIG_KPROBES @@ -168,11 +170,18 @@ struct kretprobe { raw_spinlock_t lock; }; +#define KRETPROBE_TRACE_SIZE 127 +struct kretprobe_trace { + int nr_entries; + unsigned long entries[KRETPROBE_TRACE_SIZE]; +}; + struct kretprobe_instance { struct hlist_node hlist; struct kretprobe *rp; kprobe_opcode_t *ret_addr; struct task_struct *task; + struct kretprobe_trace entry; char data[0]; }; @@ -371,6 +380,12 @@ void unregister_kretprobe(struct kretprobe *rp); int register_kretprobes(struct kretprobe **rps, int num); void unregister_kretprobes(struct kretprobe **rps, int num); +struct kretprobe_instance *current_kretprobe_instance(void); +void kretprobe_save_stack_trace(struct kretprobe_instance *ri, + struct stack_trace *trace); +void kretprobe_perf_callchain_kernel(struct kretprobe_instance *ri, + struct perf_callchain_entry_ctx *ctx); + void kprobe_flush_task(struct task_struct *tk); void recycle_rp_inst(struct kretprobe_instance *ri, struct hlist_head *head); diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c index 24a77c34e9ad..98edcd8a6987 100644 --- a/kernel/events/callchain.c +++ b/kernel/events/callchain.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "internal.h" @@ -197,9 +198,14 @@ get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user, ctx.contexts_maxed = false; if (kernel && !user_mode(regs)) { + struct kretprobe_instance *ri = current_kretprobe_instance(); + if (add_mark) perf_callchain_store_context(&ctx, PERF_CONTEXT_KERNEL); - perf_callchain_kernel(&ctx, regs); + if (ri) + kretprobe_perf_callchain_kernel(ri, &ctx); + else + perf_callchain_kernel(&ctx, regs); } if (user) { diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 90e98e233647..fca3964d18cd 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1206,6 +1206,16 @@ __releases(hlist_lock) } NOKPROBE_SYMBOL(kretprobe_table_unlock); +static bool kretprobe_hash_is_locked(struct task_struct *tsk) +{ + unsigned long hash = hash_ptr(tsk, KPROBE_HASH_BITS); + raw_spinlock_t *hlist_lock; + + hlist_lock = kretprobe_table_lock_ptr(hash); + return raw_spin_is_locked(hlist_lock); +} +NOKPROBE_SYMBOL(kretprobe_hash_is_locked); + /* * This function is called from finish_task_switch when task tk becomes dead, * so that we can recycle any function-return probe instances associated @@ -1800,6 +1810,13 @@ unsigned long __weak arch_deref_entry_point(void *entry) return (unsigned long)entry; } +static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs); + +static inline bool kprobe_is_retprobe(struct kprobe *kp) +{ + return kp->pre_handler == pre_handler_kretprobe; +} + #ifdef CONFIG_KRETPROBES /* * This kprobe pre_handler is registered with every kretprobe. When probe @@ -1826,6 +1843,8 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) hash = hash_ptr(current, KPROBE_HASH_BITS); raw_spin_lock_irqsave(&rp->lock, flags); if (!hlist_empty(&rp->free_instances)) { + struct stack_trace trace = {}; + ri = hlist_entry(rp->free_instances.first, struct kretprobe_instance, hlist); hlist_del(&ri->hlist); @@ -1834,6 +1853,11 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) ri->rp = rp; ri->task = current; + trace.entries = &ri->entry.entries[0]; + trace.max_entries = KRETPROBE_TRACE_SIZE; + save_stack_trace_regs(regs, &trace); + ri->entry.nr_entries = trace.nr_entries; + if (rp->entry_handler && rp->entry_handler(ri, regs)) { raw_spin_lock_irqsave(&rp->lock, flags); hlist_add_head(&ri->hlist, &rp->free_instances); @@ -1856,6 +1880,65 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) } NOKPROBE_SYMBOL(pre_handler_kretprobe); +/* + * Return the kretprobe_instance associated with the current_kprobe. Calling + * this is only reasonable from within a kretprobe handler context (otherwise + * return NULL). + * + * Must be called within a kretprobe_hash_lock(current, ...) context. + */ +struct kretprobe_instance *current_kretprobe_instance(void) +{ + struct kprobe *kp; + struct kretprobe *rp; + struct kretprobe_instance *ri; + struct hlist_head *head; + unsigned long hash = hash_ptr(current, KPROBE_HASH_BITS); + + kp = kprobe_running(); + if (!kp || !kprobe_is_retprobe(kp)) + return NULL; + if (WARN_ON(!kretprobe_hash_is_locked(current))) + return NULL; + + rp = container_of(kp, struct kretprobe, kp); + head = &kretprobe_inst_table[hash]; + + hlist_for_each_entry(ri, head, hlist) { + if (ri->task == current && ri->rp == rp) + return ri; + } + return NULL; +} +EXPORT_SYMBOL_GPL(current_kretprobe_instance); +NOKPROBE_SYMBOL(current_kretprobe_instance); + +void kretprobe_save_stack_trace(struct kretprobe_instance *ri, + struct stack_trace *trace) +{ + int i; + struct kretprobe_trace *krt = &ri->entry; + + for (i = trace->skip; i < krt->nr_entries; i++) { + if (trace->nr_entries >= trace->max_entries) + break; + trace->entries[trace->nr_entries++] = krt->entries[i]; + } +} + +void kretprobe_perf_callchain_kernel(struct kretprobe_instance *ri, + struct perf_callchain_entry_ctx *ctx) +{ + int i; + struct kretprobe_trace *krt = &ri->entry; + + for (i = 0; i < krt->nr_entries; i++) { + if (krt->entries[i] == ULONG_MAX) + break; + perf_callchain_store(ctx, (u64) krt->entries[i]); + } +} + bool __weak arch_kprobe_on_func_entry(unsigned long offset) { return !offset; @@ -2005,6 +2088,22 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) } NOKPROBE_SYMBOL(pre_handler_kretprobe); +struct kretprobe_instance *current_kretprobe_instance(void) +{ + return NULL; +} +EXPORT_SYMBOL_GPL(current_kretprobe_instance); +NOKPROBE_SYMBOL(current_kretprobe_instance); + +void kretprobe_save_stack_trace(struct kretprobe_instance *ri, + struct stack_trace *trace) +{ +} + +void kretprobe_perf_callchain_kernel(struct kretprobe_instance *ri, + struct perf_callchain_entry_ctx *ctx) +{ +} #endif /* CONFIG_KRETPROBES */ /* Set the kprobe gone and remove its instruction buffer. */ @@ -2241,7 +2340,7 @@ static void report_probe(struct seq_file *pi, struct kprobe *p, char *kprobe_type; void *addr = p->addr; - if (p->pre_handler == pre_handler_kretprobe) + if (kprobe_is_retprobe(p)) kprobe_type = "r"; else kprobe_type = "k"; diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index bf6f1d70484d..2210d38a4dbf 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -2590,6 +2591,7 @@ static void __ftrace_trace_stack(struct ring_buffer *buffer, struct ring_buffer_event *event; struct stack_entry *entry; struct stack_trace trace; + struct kretprobe_instance *ri = current_kretprobe_instance(); int use_stack; int size = FTRACE_STACK_ENTRIES; @@ -2626,7 +2628,9 @@ static void __ftrace_trace_stack(struct ring_buffer *buffer, trace.entries = this_cpu_ptr(ftrace_stack.calls); trace.max_entries = FTRACE_STACK_MAX_ENTRIES; - if (regs) + if (ri) + kretprobe_save_stack_trace(ri, &trace); + else if (regs) save_stack_trace_regs(regs, &trace); else save_stack_trace(&trace); @@ -2653,7 +2657,10 @@ static void __ftrace_trace_stack(struct ring_buffer *buffer, else { trace.max_entries = FTRACE_STACK_ENTRIES; trace.entries = entry->caller; - if (regs) + + if (ri) + kretprobe_save_stack_trace(ri, &trace); + else if (regs) save_stack_trace_regs(regs, &trace); else save_stack_trace(&trace); diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc new file mode 100644 index 000000000000..03146c6a1a3c --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc @@ -0,0 +1,25 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# description: Kretprobe dynamic event with a stacktrace + +[ -f kprobe_events ] || exit_unsupported # this is configurable + +echo 0 > events/enable +echo 1 > options/stacktrace + +echo 'r:teststackprobe sched_fork $retval' > kprobe_events +grep teststackprobe kprobe_events +test -d events/kprobes/teststackprobe + +clear_trace +echo 1 > events/kprobes/teststackprobe/enable +( echo "forked") +echo 0 > events/kprobes/teststackprobe/enable + +# Make sure we don't see kretprobe_trampoline and we see _do_fork. +! grep 'kretprobe' trace +grep '_do_fork' trace + +echo '-:teststackprobe' >> kprobe_events +clear_trace +test -d events/kprobes/teststackprobe && exit_fail || exit_pass From patchwork Wed Oct 31 15:25:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aleksa Sarai X-Patchwork-Id: 10662849 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7DCA613A4 for ; Wed, 31 Oct 2018 15:26:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6D1C929341 for ; Wed, 31 Oct 2018 15:26:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 600CE293BD; Wed, 31 Oct 2018 15:26:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 09E0129341 for ; Wed, 31 Oct 2018 15:26:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729561AbeKAAYx (ORCPT ); Wed, 31 Oct 2018 20:24:53 -0400 Received: from mx2.mailbox.org ([80.241.60.215]:35652 "EHLO mx2.mailbox.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728698AbeKAAYw (ORCPT ); Wed, 31 Oct 2018 20:24:52 -0400 Received: from smtp1.mailbox.org (unknown [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mx2.mailbox.org (Postfix) with ESMTPS id 62960A11EE; Wed, 31 Oct 2018 16:26:21 +0100 (CET) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter04.heinlein-hosting.de (spamfilter04.heinlein-hosting.de [80.241.56.122]) (amavisd-new, port 10030) with ESMTP id ryuJHW3PXG4w; Wed, 31 Oct 2018 16:26:20 +0100 (CET) From: Aleksa Sarai To: "Naveen N. Rao" , Anil S Keshavamurthy , "David S. Miller" , Masami Hiramatsu , Jonathan Corbet , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Steven Rostedt , Shuah Khan , Alexei Starovoitov , Daniel Borkmann Cc: Aleksa Sarai , Aleksa Sarai , Christian Brauner , Brendan Gregg , netdev@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v2 2/2] trace: remove kretprobed checks Date: Thu, 1 Nov 2018 02:25:43 +1100 Message-Id: <20181031152543.12138-3-cyphar@cyphar.com> In-Reply-To: <20181031152543.12138-1-cyphar@cyphar.com> References: <20181031152543.12138-1-cyphar@cyphar.com> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This is effectively a reversion of commit 76094a2cf46e ("ftrace: distinguish kretprobe'd functions in trace logs"), as the checking of kretprobe_trampoline *for tracing* is no longer necessary with the new kretprobe stack trace changes. Signed-off-by: Aleksa Sarai --- kernel/trace/trace_output.c | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index 6e6cc64faa38..951de16bd4fd 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c @@ -321,36 +321,14 @@ int trace_output_call(struct trace_iterator *iter, char *name, char *fmt, ...) } EXPORT_SYMBOL_GPL(trace_output_call); -#ifdef CONFIG_KRETPROBES -static inline const char *kretprobed(const char *name) -{ - static const char tramp_name[] = "kretprobe_trampoline"; - int size = sizeof(tramp_name); - - if (strncmp(tramp_name, name, size) == 0) - return "[unknown/kretprobe'd]"; - return name; -} -#else -static inline const char *kretprobed(const char *name) -{ - return name; -} -#endif /* CONFIG_KRETPROBES */ - static void seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long address) { char str[KSYM_SYMBOL_LEN]; #ifdef CONFIG_KALLSYMS - const char *name; - kallsyms_lookup(address, NULL, NULL, NULL, str); - - name = kretprobed(str); - - if (name && strlen(name)) { - trace_seq_printf(s, fmt, name); + if (strlen(str)) { + trace_seq_printf(s, fmt, str); return; } #endif @@ -364,13 +342,9 @@ seq_print_sym_offset(struct trace_seq *s, const char *fmt, { char str[KSYM_SYMBOL_LEN]; #ifdef CONFIG_KALLSYMS - const char *name; - sprint_symbol(str, address); - name = kretprobed(str); - - if (name && strlen(name)) { - trace_seq_printf(s, fmt, name); + if (strlen(str)) { + trace_seq_printf(s, fmt, str); return; } #endif