linux-kernel - Re: [PATCH 1/3] ftrace: Fix updating of filters for shared global

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: <54B774DF.7010209@hitachi.com>
Date:	Thu, 15 Jan 2015 17:05:51 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, vvs@...ru,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>, x86-ml <x86@...nel.org>,
	stable@...r.kernel.org
Subject: Re: [PATCH 1/3] ftrace: Fix updating of filters for shared global_ops
 filters

(2015/01/15 0:39), Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.org>
> 
> As the set_ftrace_filter affects both the function tracer as well as the
> function graph tracer, the ops that represent each have a shared
> ftrace_ops_hash structure. This allows both to be updated when the filter
> files are updated.
> 
> But if function graph is enabled and the global_ops (function tracing) ops
> is not, then it is possible that the filter could be changed without the
> update happening for the function graph ops. This will cause the changes
> to not take place and may even cause a ftrace_bug to occur as it could mess
> with the trampoline accounting.
> 
> The solution is to check if the ops uses the shared global_ops filter and
> if the ops itself is not enabled, to check if there's another ops that is
> enabled and also shares the global_ops filter. In that case, the
> modification still needs to be executed.
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>

Thank you,

> Cc: stable@...r.kernel.org # 3.17+
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
>  kernel/trace/ftrace.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 929a733d302e..2b35d0ba578d 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4008,8 +4008,32 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
>  static void ftrace_ops_update_code(struct ftrace_ops *ops,
>  				   struct ftrace_hash *old_hash)
>  {
> -	if (ops->flags & FTRACE_OPS_FL_ENABLED && ftrace_enabled)
> +	struct ftrace_ops *op;
> +
> +	if (!ftrace_enabled)
> +		return;
> +
> +	if (ops->flags & FTRACE_OPS_FL_ENABLED) {
>  		ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS, old_hash);
> +		return;
> +	}
> +
> +	/*
> +	 * If this is the shared global_ops filter, then we need to
> +	 * check if there is another ops that shares it, is enabled.
> +	 * If so, we still need to run the modify code.
> +	 */
> +	if (ops->func_hash != &global_ops.local_hash)
> +		return;
> +
> +	do_for_each_ftrace_op(op, ftrace_ops_list) {
> +		if (op->func_hash == &global_ops.local_hash &&
> +		    op->flags & FTRACE_OPS_FL_ENABLED) {
> +			ftrace_run_modify_code(op, FTRACE_UPDATE_CALLS, old_hash);
> +			/* Only need to do this once */
> +			return;
> +		}
> +	} while_for_each_ftrace_op(op);
>  }
>  
>  static int

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@...achi.com


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