linux-kernel - Re: [RFC/PATCH] ftrace: add set_graph

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: <20131011082204.5c6de75d@gandalf.local.home>
Date:	Fri, 11 Oct 2013 08:22:04 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Heiko Carstens <heiko.carstens@...ibm.com>
Cc:	Namhyung Kim <namhyung@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Namhyung Kim <namhyung.kim@....com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>
Subject: Re: [RFC/PATCH] ftrace: add set_graph_notrace filter

On Fri, 11 Oct 2013 09:21:56 +0200
Heiko Carstens <heiko.carstens@...ibm.com> wrote:

> 
> Steven Rostedt noted that s390 is the only architecture which calls
> ftrace_push_return_trace() before ftrace_graph_entry() and therefore has

I don't know if s390 is the only arch. I didn't finish reading all the
arch specific code to make sure.

> the small advantage that trace.depth gets initialized automatically.
> 
> However this small advantage isn't worth the difference and possible subtle
> breakage that may result from this.
> So change s390 to have the same function call order like all other
> architectures: first ftrace_graph_entry(), then ftrace_push_return_trace()
> 
> Reported-by: Steven Rostedt <rostedt@...dmis.org>
> Signed-off-by: Heiko Carstens <heiko.carstens@...ibm.com>
> ---
>  arch/s390/kernel/ftrace.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> index 1014ad5..224db03 100644
> --- a/arch/s390/kernel/ftrace.c
> +++ b/arch/s390/kernel/ftrace.c
> @@ -151,14 +151,13 @@ unsigned long __kprobes prepare_ftrace_return(unsigned long parent,
>  	if (unlikely(atomic_read(&current->tracing_graph_pause)))
>  		goto out;
>  	ip = (ip & PSW_ADDR_INSN) - MCOUNT_INSN_SIZE;
> -	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0) == -EBUSY)
> -		goto out;
>  	trace.func = ip;
> +	trace.depth = current->curr_ret_stack + 1;
>  	/* Only trace if the calling function expects to. */
> -	if (!ftrace_graph_entry(&trace)) {
> -		current->curr_ret_stack--;
> +	if (!ftrace_graph_entry(&trace))
> +		goto out;
> +	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0) == -EBUSY)
>  		goto out;
> -	}

Hmm, if this code is so identical among the archs, I may create a
generic inline function to do the work. That way any change to the flow
can be done for all archs in one place.

Thanks!

-- Steve

>  	parent = (unsigned long) return_to_handler;
>  out:
>  	return parent;

--
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/