linux-kernel - Re: [tip:tracing/ftrace] ftrace: don't try to __ftrace_replace_code on !FTRACE_FL

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.00.0903130757450.18396@gandalf.stny.rr.com>
Date:	Fri, 13 Mar 2009 08:02:56 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org,
	zhaolei@...fujitsu.com, tglx@...utronix.de, mingo@...e.hu
cc:	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:tracing/ftrace] ftrace: don't try to __ftrace_replace_code
 on !FTRACE_FL_CONVERTED rec



On Fri, 13 Mar 2009, Zhaolei wrote:

> Commit-ID:  fa9d13cf135efbd454453a53b6299976bea245a9
> Gitweb:     http://git.kernel.org/tip/fa9d13cf135efbd454453a53b6299976bea245a9
> Author:     Zhaolei <zhaolei@...fujitsu.com>
> AuthorDate: Fri, 13 Mar 2009 17:16:34 +0800
> Commit:     Ingo Molnar <mingo@...e.hu>
> CommitDate: Fri, 13 Mar 2009 10:25:06 +0100
> 
> ftrace: don't try to __ftrace_replace_code on !FTRACE_FL_CONVERTED rec
> 
> Do __ftrace_replace_code for !FTRACE_FL_CONVERTED rec will always
> fail, we should ignore this rec.

Is this also by code review or did you actually hit it?

This code is very sensitive, and changing must be done extremely 
carefully.

> 
> Signed-off-by: Zhao Lei <zhaolei@...fujitsu.com>
> Cc: "Steven Rostedt ;" <rostedt@...dmis.org>
> LKML-Reference: <49BA2472.4060206@...fujitsu.com>
> Signed-off-by: Ingo Molnar <mingo@...e.hu>
> 
> 
> ---
>  kernel/trace/ftrace.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 26c45aa..08f4a62 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -532,11 +532,12 @@ static void ftrace_replace_code(int enable)
>  
>  	do_for_each_ftrace_rec(pg, rec) {
>  		/*
> -		 * Skip over free records and records that have
> -		 * failed.
> +		 * Skip over free records, records that have
> +		 * failed and not converted.
>  		 */
>  		if (rec->flags & FTRACE_FL_FREE ||
> -		    rec->flags & FTRACE_FL_FAILED)
> +		    rec->flags & FTRACE_FL_FAILED ||
> +		    rec->flags & FTRACE_FL_CONVERTED)
>  			continue;
>  
>  		/* ignore updates to this record's mcount site */
> @@ -548,7 +549,7 @@ static void ftrace_replace_code(int enable)
>  		}
>  
>  		failed = __ftrace_replace_code(rec, enable);
> -		if (failed && (rec->flags & FTRACE_FL_CONVERTED)) {
> +		if (failed) {

Note, if we get here when a record does not have FL_CONVERTED set, then
something already went wrong and tracing should have been disabled.
The check here was simply to prevent mulitple errors from being printed 
out.

I'm just getting ready to travel, but maybe I'll look into this code a bit
more, and see what reasons we can get here without that CONVERTED flag
being set. Perhaps the best thing to do is exit the loop?

-- Steve

>  			rec->flags |= FTRACE_FL_FAILED;
>  			if ((system_state == SYSTEM_BOOTING) ||
>  			    !core_kernel_text(rec->ip)) {
> 
--
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/