linux-kernel - Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set via debugfs

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: <20140312135059.2d497b58@gandalf.local.home>
Date:	Wed, 12 Mar 2014 13:50:59 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	"Frank Ch. Eigler" <fche@...hat.com>, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Berg <johannes.berg@...el.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	lttng-dev <lttng-dev@...ts.lttng.org>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not
 set via debugfs

On Wed, 12 Mar 2014 16:39:55 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:

> > Are you telling me it's not possible to delete the entire probe?
> 
> The ownership flow is the following:
> 
> 1) Tracer creates name, probe, data objects. The probe can be typically
>    code within a probe provider module, which needs to have a reference
>    count incremented. The name and data objects can be dynamically allocated,
>    or in some special cases part of a probe provider module (again with
>    refcount incremented).
> 
> 2) The tracer registers the tracepoint probe. If registration returns 0,
>    the tracer should not free those elements until it calls tracepoint
>    probe unregister for that (name, probe, data) tuple.
> 
> 3) Tracer calls tracepoint probe unregister for the (name, probe, data) tuple.

We can make the registered tracepoint up the mod count to prevent it
from unloading. But that probably defeats the purpose.

> 
> 4) Tracer calls tracepoint_synchronize_unregister() to ensure quiescence
>    of tracepoint call sites wrt the probe that has just been unregistered.

Tracepoints should all be disabled when the module is unloaded. If you
have a tracepoint callback still being called when the module is
unloaded then something is seriously wrong. That means the callback
will go back to the trace_foo() call which is in the module code and
will no longer exist. Kernel panic is the result.

> 
> 5) Tracer can free/unref the probe provider module.

I'm a bit confused at what you are doing. As this is totally unrelated
to anything that happens in the kernel.

> 
> > 
> > What I'm proposing is to do what the trace events do. Delete everything
> > associated to the tracepoints associated to the module.
> 
> Is your intent to have a module "going" notifier in tracepoint.c managing
> ownership of objects it does not own ? If not, I guess I'm not understanding
> your proposal fully.

Well, actually that's exactly what the trace_event code does. It
disables any event of the module that happens to be enabled.

Look at event_remove() in kernel/trace/trace_events.c

On module unload, the events are destroyed.

Thus, what your module should do, is exactly what event_remove() does.
On module unload, you unregister any of the tracepoints that were
registered. Just like any other module resource. If you request a
resource on the behalf of a module, it is up to you to free it when the
module is unloaded.

The tracepoint code will just destroy what it set up when the module
was loaded. It's up to your module to clean up the allocations that you
made when the module was loaded on unload. Just like we do for all
other resources.

Mathieu, stop thinking that tracepoints are special. They are not.

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