* [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
@ 2009-09-28 15:43 Matt Fleming
2009-09-29 9:22 ` Frederic Weisbecker
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Matt Fleming @ 2009-09-28 15:43 UTC (permalink / raw
To: linux-kernel; +Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar
From: Matt Fleming <matthew.fleming@imgtec.com>
When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
__ftrace_trace_function contains the current trace function, not
ftrace_trace_function. In ftrace_update_pid_func() we currently
incorrectly assign the value of ftrace_trace_function to
__ftrace_trace_funcion before returning.
Without this patch it is possible to execute an infinite loop whereby
ftrace_test_stop_func() calls __ftrace_trace_function, which was
assigned ftrace_test_stop_func() in ftrace_update_pid_func().
Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
---
kernel/trace/ftrace.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 25edd5c..d9ba6d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -225,7 +225,11 @@ static void ftrace_update_pid_func(void)
if (ftrace_trace_function == ftrace_stub)
return;
+#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
func = ftrace_trace_function;
+#else
+ func = __ftrace_trace_function;
+#endif
if (ftrace_pid_trace) {
set_ftrace_pid_function(func);
--
1.6.3.GIT
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
2009-09-28 15:43 [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func() Matt Fleming
@ 2009-09-29 9:22 ` Frederic Weisbecker
2009-09-29 9:44 ` Matt Fleming
2009-09-30 8:41 ` Steven Rostedt
2009-09-29 9:51 ` Matt Fleming
2009-10-01 6:48 ` [tip:tracing/urgent] tracing: Fix infinite recursion " tip-bot for Matt Fleming
2 siblings, 2 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2009-09-29 9:22 UTC (permalink / raw
To: Matt Fleming, Steven Rostedt; +Cc: linux-kernel, Ingo Molnar
On Mon, Sep 28, 2009 at 04:43:01PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matthew.fleming@imgtec.com>
>
> When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
> __ftrace_trace_function contains the current trace function, not
> ftrace_trace_function. In ftrace_update_pid_func() we currently
> incorrectly assign the value of ftrace_trace_function to
> __ftrace_trace_funcion before returning.
>
> Without this patch it is possible to execute an infinite loop whereby
> ftrace_test_stop_func() calls __ftrace_trace_function, which was
> assigned ftrace_test_stop_func() in ftrace_update_pid_func().
>
> Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
> ---
> kernel/trace/ftrace.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 25edd5c..d9ba6d9 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -225,7 +225,11 @@ static void ftrace_update_pid_func(void)
> if (ftrace_trace_function == ftrace_stub)
> return;
>
> +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
> func = ftrace_trace_function;
> +#else
> + func = __ftrace_trace_function;
> +#endif
I don't understand how it can do that infinite loop.
It doesn't seem the following can happen:
func = ftrace_trace_function //which is ftrace_test_stop_func
_ftrace_trace_function = func
Because we can sum up the path like that:
func = ftrace_trace_function;
...
#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
ftrace_trace_function = func;
#else
__ftrace_trace_function = func;
#endif
So if we are in CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST,
that doesn't seem to harm, although it seems that what we want is
func = __ftrace_trace_function
...
__ftrace_trace_function = func
Because we want to always keep ftrace_test_stop_func as the top level
wrapper. Which would be what does the above, plus the fact we avoid any
recursivity.
But it's possible I'm missing the point...
Also, Steve, is this an option that is still useful? It's even not
in Kconfig.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
2009-09-29 9:22 ` Frederic Weisbecker
@ 2009-09-29 9:44 ` Matt Fleming
2009-09-30 8:41 ` Steven Rostedt
1 sibling, 0 replies; 7+ messages in thread
From: Matt Fleming @ 2009-09-29 9:44 UTC (permalink / raw
To: Frederic Weisbecker; +Cc: Steven Rostedt, linux-kernel, Ingo Molnar
On Tue, Sep 29, 2009 at 11:22:47AM +0200, Frederic Weisbecker wrote:
> On Mon, Sep 28, 2009 at 04:43:01PM +0100, Matt Fleming wrote:
> > From: Matt Fleming <matthew.fleming@imgtec.com>
> >
> > When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
> > __ftrace_trace_function contains the current trace function, not
> > ftrace_trace_function. In ftrace_update_pid_func() we currently
> > incorrectly assign the value of ftrace_trace_function to
> > __ftrace_trace_funcion before returning.
> >
> > Without this patch it is possible to execute an infinite loop whereby
> > ftrace_test_stop_func() calls __ftrace_trace_function, which was
> > assigned ftrace_test_stop_func() in ftrace_update_pid_func().
> >
> > Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
> > ---
> > kernel/trace/ftrace.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 25edd5c..d9ba6d9 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -225,7 +225,11 @@ static void ftrace_update_pid_func(void)
> > if (ftrace_trace_function == ftrace_stub)
> > return;
> >
> > +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
> > func = ftrace_trace_function;
> > +#else
> > + func = __ftrace_trace_function;
> > +#endif
>
>
>
> I don't understand how it can do that infinite loop.
> It doesn't seem the following can happen:
>
> func = ftrace_trace_function //which is ftrace_test_stop_func
>
> _ftrace_trace_function = func
>
Here is the unpatched version of ftrace_update_pid_func() from
kernel/trace/ftrace.c
Before calling:
ftrace_trace_function = ftrace_test_stop_func
__ftrace_trace_Function = real tracing function
static void ftrace_update_pid_func(void)
{
ftrace_func_t func;
if (ftrace_trace_function == ftrace_stub)
return;
func = ftrace_trace_function;
if (ftrace_pid_trace) {
set_ftrace_pid_function(func);
func = ftrace_pid_func;
} else {
if (func == ftrace_pid_func)
func = ftrace_pid_function;
}
#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
ftrace_trace_function = func;
#else
__ftrace_trace_function = func;
#endif
}
After calling:
ftrace_trace_function = ftrace_test_stop_func
__ftrace_trace_function = ftrace_test_stop_func
Then look what happens next time ftrace_test_stop_func() is called via
ftrace_call,
#ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
/*
* For those archs that do not test ftrace_trace_stop in their
* mcount call site, we need to do it from C.
*/
static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip)
{
if (function_trace_stop)
return;
__ftrace_trace_function(ip, parent_ip);
}
#endif
Here we're going to recurse infinitely because,
__ftrace_trace_function = ftrace_test_stop_func
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
2009-09-28 15:43 [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func() Matt Fleming
2009-09-29 9:22 ` Frederic Weisbecker
@ 2009-09-29 9:51 ` Matt Fleming
2009-10-01 6:48 ` [tip:tracing/urgent] tracing: Fix infinite recursion " tip-bot for Matt Fleming
2 siblings, 0 replies; 7+ messages in thread
From: Matt Fleming @ 2009-09-29 9:51 UTC (permalink / raw
To: linux-kernel; +Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar
On Mon, Sep 28, 2009 at 04:43:01PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matthew.fleming@imgtec.com>
>
> When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
> __ftrace_trace_function contains the current trace function, not
> ftrace_trace_function. In ftrace_update_pid_func() we currently
> incorrectly assign the value of ftrace_trace_function to
> __ftrace_trace_funcion before returning.
>
> Without this patch it is possible to execute an infinite loop whereby
> ftrace_test_stop_func() calls __ftrace_trace_function, which was
> assigned ftrace_test_stop_func() in ftrace_update_pid_func().
>
Oops..
I just realised I've used the phrase "infinite loop" when what I meant
to say was "infinite recursion". Sorry, brain fart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
2009-09-29 9:22 ` Frederic Weisbecker
2009-09-29 9:44 ` Matt Fleming
@ 2009-09-30 8:41 ` Steven Rostedt
2009-09-30 8:51 ` Matt Fleming
1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2009-09-30 8:41 UTC (permalink / raw
To: Frederic Weisbecker; +Cc: Matt Fleming, linux-kernel, Ingo Molnar
On Tue, 2009-09-29 at 11:22 +0200, Frederic Weisbecker wrote:
> On Mon, Sep 28, 2009 at 04:43:01PM +0100, Matt Fleming wrote:
> > From: Matt Fleming <matthew.fleming@imgtec.com>
> >
> > When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
> > __ftrace_trace_function contains the current trace function, not
> > ftrace_trace_function. In ftrace_update_pid_func() we currently
> > incorrectly assign the value of ftrace_trace_function to
> > __ftrace_trace_funcion before returning.
> >
> > Without this patch it is possible to execute an infinite loop whereby
> > ftrace_test_stop_func() calls __ftrace_trace_function, which was
> > assigned ftrace_test_stop_func() in ftrace_update_pid_func().
> >
> > Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
> > ---
> > kernel/trace/ftrace.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 25edd5c..d9ba6d9 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -225,7 +225,11 @@ static void ftrace_update_pid_func(void)
> > if (ftrace_trace_function == ftrace_stub)
> > return;
> >
> > +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
> > func = ftrace_trace_function;
> > +#else
> > + func = __ftrace_trace_function;
> > +#endif
>
>
>
> I don't understand how it can do that infinite loop.
> It doesn't seem the following can happen:
>
> func = ftrace_trace_function //which is ftrace_test_stop_func
>
> _ftrace_trace_function = func
>
> Because we can sum up the path like that:
>
> func = ftrace_trace_function;
>
> ...
>
>
> #ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
> ftrace_trace_function = func;
> #else
> __ftrace_trace_function = func;
> #endif
>
> So if we are in CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST,
> that doesn't seem to harm, although it seems that what we want is
>
> func = __ftrace_trace_function
>
> ...
>
> __ftrace_trace_function = func
>
> Because we want to always keep ftrace_test_stop_func as the top level
> wrapper. Which would be what does the above, plus the fact we avoid any
> recursivity.
>
> But it's possible I'm missing the point...
>
> Also, Steve, is this an option that is still useful? It's even not
> in Kconfig.
>
Yeah, Matt found a real bug, and yes it will infinitely recurse. The
thing is that the function can call itself which will continue to do so.
As for the Kconfig, it's something that currently only x86 selects,
because x86 does the test in entry.S.
Matt,
Thanks, I'll pull this in as soon as I get back from Dresden.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()
2009-09-30 8:41 ` Steven Rostedt
@ 2009-09-30 8:51 ` Matt Fleming
0 siblings, 0 replies; 7+ messages in thread
From: Matt Fleming @ 2009-09-30 8:51 UTC (permalink / raw
To: Steven Rostedt; +Cc: Frederic Weisbecker, linux-kernel, Ingo Molnar
On Wed, Sep 30, 2009 at 04:41:15AM -0400, Steven Rostedt wrote:
>
> Matt,
>
> Thanks, I'll pull this in as soon as I get back from Dresden.
>
> -- Steve
Thanks Steve!
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:tracing/urgent] tracing: Fix infinite recursion in ftrace_update_pid_func()
2009-09-28 15:43 [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func() Matt Fleming
2009-09-29 9:22 ` Frederic Weisbecker
2009-09-29 9:51 ` Matt Fleming
@ 2009-10-01 6:48 ` tip-bot for Matt Fleming
2 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Matt Fleming @ 2009-10-01 6:48 UTC (permalink / raw
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, fweisbec, rostedt, tglx,
matthew.fleming, mingo
Commit-ID: 33974093c024f08caadd2fc71a83bd811ed1831d
Gitweb: http://git.kernel.org/tip/33974093c024f08caadd2fc71a83bd811ed1831d
Author: Matt Fleming <matthew.fleming@imgtec.com>
AuthorDate: Mon, 28 Sep 2009 16:43:01 +0100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 1 Oct 2009 08:19:24 +0200
tracing: Fix infinite recursion in ftrace_update_pid_func()
When CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST is enabled
__ftrace_trace_function contains the current trace function, not
ftrace_trace_function.
In ftrace_update_pid_func() we currently incorrectly assign the
value of ftrace_trace_function to __ftrace_trace_funcion before
returning.
Without this patch it is possible to execute an infinite recursion
whereby ftrace_test_stop_func() calls __ftrace_trace_function,
which was assigned ftrace_test_stop_func() in
ftrace_update_pid_func().
Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1254152581-18347-1-git-send-email-matt@console-pimps.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/trace/ftrace.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 46592fe..3724756 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -225,7 +225,11 @@ static void ftrace_update_pid_func(void)
if (ftrace_trace_function == ftrace_stub)
return;
+#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
func = ftrace_trace_function;
+#else
+ func = __ftrace_trace_function;
+#endif
if (ftrace_pid_trace) {
set_ftrace_pid_function(func);
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-10-01 6:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-28 15:43 [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func() Matt Fleming
2009-09-29 9:22 ` Frederic Weisbecker
2009-09-29 9:44 ` Matt Fleming
2009-09-30 8:41 ` Steven Rostedt
2009-09-30 8:51 ` Matt Fleming
2009-09-29 9:51 ` Matt Fleming
2009-10-01 6:48 ` [tip:tracing/urgent] tracing: Fix infinite recursion " tip-bot for Matt Fleming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).