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: <20140312153006.7acea4a8@gandalf.local.home>
Date:	Wed, 12 Mar 2014 15:30:06 -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 14:58:02 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> 
> Two modules should not have the same name. Is there any duplicate
> tracepoints you are aware of. Namespace collisions in tracepoints
> should be avoided, as that would cause people to trace things they did
> not intend on tracing.
> 
> That should be a new patch as well. Enforce unique tracepoint names.

This may be why you are not understanding what I want. It's the way
things are implemented today, which I believe are wrong. I see what you
did. You have probes that are registered, and tracepoints that are
where the code lies. You just add and remove probes from a hash list,
and then you loop through all the tracepoints seeing if the iter->name
matches a probe->name.

I'm fine with keeping the probe separate, but there really should be
no more than just a one to one mapping between probes and tracepoints.
Have the probe point to the matching tracepoint. The probe is
registered, it enables the tracepoint static key, when it's ref count
goes to zero, it disables the tracepoint static key. We can get rid of
that loop then, as well as the duplicate names between probes and
tracepoints.

Here's the steps we should take:

1) Prevent duplicate tracepoints. They are just namespace collisions
that we already try to avoid. How to do this? We may need to add a
hlist_node to the tracepoint structure, and keep them in a hash by name.
Check for collisions when the name is added to the hash.

2) Change the way tracepoints are enabled. Do not do a loop of all
tracepoints, but instead have the first probe of a tracepoint enable
it, and the last one to disable it. This would require a pointer from
the probe to the tracepoint it represents. Again, it should not
represent more than one.

3) On module unload, it would be the responsibility of the user to
unload all the tracepoints that may have been enabled for a module. We
can add a mod pointer in the probe to make this easier, as well as to
the tp_module structure.

The way tracepoints are today are to handle two completely different
tracepoints with the same name. That should be avoided, and will make
things much less complex.

Then you can easily handle the accounting of modules loading and
unloading in your module, and the tracepoint code will match what the
rest of the kernel does for resource management.

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