linux-kernel - Re: [PATCH] tracing/profile: Fix profile_disable vs module

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: <alpine.DEB.2.00.0908271125030.11745@gandalf.stny.rr.com>
Date:	Thu, 27 Aug 2009 11:51:29 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
cc:	Li Zefan <lizf@...fujitsu.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing/profile: Fix profile_disable vs module_unload

On Thu, 27 Aug 2009, Mathieu Desnoyers wrote:

> * Steven Rostedt (rostedt@...dmis.org) wrote:
> > 
> > On Thu, 27 Aug 2009, Mathieu Desnoyers wrote:
> > 
> > > Looks good. Just don't forget to eventually add the "synchronize" calls
> > > between tracepoint unregistration and the removal of their module. There
> > > is a race condition in the way you do it currently.
> > 
> > I'm trying to figure out the race here. What will disappear in the 
> > tracepoint? Could you give a brief example of the issue.
> 
> Sure,
> 
> Let's say we have a tracepoint in module instrumented.c, and a probe in
> module probe.c. The probe is registered by module probe.c init through
> the tracepoint infrastructure to connect to the tracepoint in
> instrumented.c. Unregistration is done in probe.c module exit.
> 
> As the instrumented code get executed (let's say periodically), it calls
> the connected probe. Preemption is disabled around the call.
> 
> If you unload the probe.c module, the module exit will unregister the
> probe, but the probe code can still be in use by another CPU. You have
> to wait for quiescent state with the tracepoint synchronize (which is
> just a wrapper over synchronize_sched() before you are allowed to
> complete module unload. Otherwise, you will end up reclaiming module
> memory that is still used by probe execution.
> 
> A test-case for this would be to create a probe with a delay in it, and
> an instrumented module calling the instrumentation in a loop. On a SMP
> system, running the instrumented code and probe load/unload in loops
> should trigger this race.

Thanks for the explanation. So let me see if I get this correct.

For this race to occur, the probe (code that hooks to the tracepoint) must 
be in module that does not contain the tracepoint. We don't even need more 
than one module, this could occur even with a core tracepoint. If a module 
registers it, if it unregisters before unloading, the tracepoint may be 
hit before the unregister and executing while the module is unloading.

I don't think we need to worry about this with the case of TRACE_EVENT and 
ftrace.h. The reason is that the trace point and probes are always in the 
same location. The MACROS set up the probe code with the modules. Thus, to 
remove the module, you must also remove the tracepoint itself along with 
the probe. If you can be executing in the probe, then you must have hit 
the trace point. If you hit the trace point, then you are executing code 
inside the module you are removing, which is a bug in the module code 
itself.

Using the ftrace.h MACROS limits the use of tracepoints and this race 
does not exist. I feel we are safe not needing to have the 
tracepoint_synchronize_unregister within the ftrace.h code.

-- Steve

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