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
| ||
|
Date: Thu, 21 Jan 2010 21:05:17 -0500 From: Steven Rostedt <rostedt@...dmis.org> To: Frederic Weisbecker <fweisbec@...il.com> Cc: Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>, Li Zefan <lizf@...fujitsu.com>, Lai Jiangshan <laijs@...fujitsu.com>, Mathieu Desnoyers <compudj@...stal.dyndns.org> Subject: Re: [RFC PATCH 03/10] ftrace: Drop the ftrace_profile_enabled checks in tracing hot path On Fri, 2010-01-22 at 02:16 +0100, Frederic Weisbecker wrote: > Every time we enter the function profiler tracing callbacks, we first > check if the function profiling is enabled. > > This check is useless because we register the function graph > callbacks only after the hashlist has been initialized. Unfortunately, since the previous patch is incorrect, it makes this one buggy too. If you remove the check to ftrace_profile_enabled, the call to the profiled code could have been preempted and pending to be called. Stop machine may remove all calls to the tracing, but it only affects new hits. Pending hits may still exist. If you remove this check, and the user re-enables the profiling, then all PER_CPU hashs will be reset. If in the process of this happening, the task with the pending trace wakes up, it may access the PER_CPU list and corrupt it. Now for the reason I Cc'd Paul and Mathieu... If we had a synchronize_sched() like function that would wait and return when all preempted tasks have been scheduled again and went to either userspace or called schedule directly, then we could actually do this. After unregistering the function graph trace, you call this "synchronize_tasks()" and it will guarantee that all currently preempted tasks have either went to userspace or have called schedule() directly. Then it would be safe to remove this check. -- Steve > > Signed-off-by: Frederic Weisbecker <fweisbec@...il.com> > Cc: Steven Rostedt <rostedt@...dmis.org> > Cc: Li Zefan <lizf@...fujitsu.com> > Cc: Lai Jiangshan <laijs@...fujitsu.com> > --- > kernel/trace/ftrace.c | 7 ++----- > 1 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 94117ec..f258f67 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -381,13 +381,10 @@ function_profile_call(unsigned long ip, unsigned long parent_ip) > struct func_node *rec; > unsigned long flags; > > - if (!ftrace_profile_enabled) > - return; > - > local_irq_save(flags); > > hlist = &__get_cpu_var(func_hlist_cpu); > - if (!hlist->hash || !ftrace_profile_enabled) > + if (!hlist->hash) > goto out; > > rec = function_find_hlist_node(hlist, ip); > @@ -418,7 +415,7 @@ static void profile_graph_return(struct ftrace_graph_ret *trace) > > local_irq_save(flags); > hlist = &__get_cpu_var(func_hlist_cpu); > - if (!hlist->hash || !ftrace_profile_enabled) > + if (!hlist->hash) > goto out; > > calltime = trace->rettime - trace->calltime; -- 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