lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090929092245.GA5057@nowhere>
Date:	Tue, 29 Sep 2009 11:22:47 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Matt Fleming <matt@...sole-pimps.org>,
	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH] tracing: Fix infinite loop in ftrace_update_pid_func()

On Mon, Sep 28, 2009 at 04:43:01PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matthew.fleming@...tec.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@...tec.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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ