From: Eric Dumazet <eric.dumazet@gmail.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>, LKML <linux-kernel@vger.kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Paul Mackerras <paulus@samba.org>,
David Miller <davem@davemloft.net>,
Archs <linux-arch@vger.kernel.org>
Subject: Re: [PATCH] perf: Fix unsafe frame rewinding with hot regs fetching
Date: Thu, 08 Apr 2010 15:52:26 +0200 [thread overview]
Message-ID: <1270734746.2215.56.camel@edumazet-laptop> (raw)
In-Reply-To: <20100408123209.GA6096@nowhere>
Le jeudi 08 avril 2010 à 14:32 +0200, Frederic Weisbecker a écrit :
>
> Can you please test this fix?
>
> Thanks.
>
> ---
> From 60d5c4e8498efc4a01abceef54ad3bc91993bf41 Mon Sep 17 00:00:00 2001
> From: Frederic Weisbecker <fweisbec@gmail.com>
> Date: Thu, 8 Apr 2010 14:05:50 +0200
> Subject: [PATCH] perf: Fix unsafe frame rewinding with hot regs fetching
>
> When we fetch the hot regs and rewind to the nth caller, it
> might happen that we dereference a frame pointer outside the
> kernel stack boundaries, like in this example:
>
> perf_trace_sched_switch+0xd5/0x120
> schedule+0x6b5/0x860
> retint_careful+0xd/0x21
>
> Since we directly dereference a userspace frame pointer here while
> rewinding behind retint_careful, this may end up in a crash.
>
> Fix this by simply using probe_kernel_address() when we rewind the
> frame pointer.
>
> This issue will have a much more proper fix in the next version of the
> perf_arch_fetch_caller_regs() API that will only need to rewind to the
> first caller.
>
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Archs <linux-arch@vger.kernel.org>
> ---
> arch/x86/kernel/dumpstack.h | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/dumpstack.h b/arch/x86/kernel/dumpstack.h
> index e39e771..e1a93be 100644
> --- a/arch/x86/kernel/dumpstack.h
> +++ b/arch/x86/kernel/dumpstack.h
> @@ -14,6 +14,8 @@
> #define get_bp(bp) asm("movq %%rbp, %0" : "=r" (bp) :)
> #endif
>
> +#include <linux/uaccess.h>
> +
> extern void
> show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
> unsigned long *stack, unsigned long bp, char *log_lvl);
> @@ -42,8 +44,10 @@ static inline unsigned long rewind_frame_pointer(int n)
> get_bp(frame);
>
> #ifdef CONFIG_FRAME_POINTER
> - while (n--)
> - frame = frame->next_frame;
> + while (n--) {
> + if (probe_kernel_address(&frame->next_frame, frame))
> + break;
> + }
> #endif
>
> return (unsigned long)frame;
Thanks, no more crash :)
Tested-by: Eric Dumazet <eric.dumazet@gmail.com>
next prev parent reply other threads:[~2010-04-08 13:52 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-26 1:52 [PATCH 0/7] perf updates and fixes Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 1/7] perf: Drop the frame reliablity check Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 2/7] perf: Fetch hot regs from the template caller Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 3/7] x86: Unify dumpstack.h and stacktrace.h Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 4/7] perf: Move perf_arch_fetch_caller_regs into a macro Frederic Weisbecker
2010-03-26 1:52 ` Frederic Weisbecker
2010-03-26 1:52 ` Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 5/7] perf: Make perf_fetch_caller_regs rewind to the first caller only Frederic Weisbecker
2010-03-26 1:52 ` Frederic Weisbecker
2010-03-26 1:52 ` Frederic Weisbecker
2010-04-08 9:57 ` [BUG perf] perf_fetch_caller_regs / rewind_frame_pointer can panic Eric Dumazet
2010-04-08 10:59 ` Frederic Weisbecker
2010-04-08 12:32 ` [PATCH] perf: Fix unsafe frame rewinding with hot regs fetching Frederic Weisbecker
2010-04-08 12:32 ` Frederic Weisbecker
2010-04-08 13:52 ` Eric Dumazet [this message]
2010-04-08 17:31 ` [GIT PULL] perf fix Frederic Weisbecker
2010-04-13 22:51 ` Ingo Molnar
2010-03-26 1:52 ` [PATCH 6/7] perf: Use hot regs with software sched/migrate events Frederic Weisbecker
2010-03-26 1:52 ` [PATCH 7/7] perf: Correctly align perf event tracing buffer Frederic Weisbecker
2010-03-26 6:02 ` [PATCH 0/7] perf updates and fixes Paul Mackerras
2010-03-26 7:58 ` Ingo Molnar
2010-03-26 17:38 ` Frederic Weisbecker
2010-03-26 17:45 ` Frederic Weisbecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1270734746.2215.56.camel@edumazet-laptop \
--to=eric.dumazet@gmail.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@redhat.com \
--cc=davem@davemloft.net \
--cc=fweisbec@gmail.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.