Re: [PATCH 3/3] Make jprobes a little safer for users

On Tue, 26 Jun 2007 16:03:58 +1000 Michael Ellerman [off-list ref] wrote:

On Tue, 2007-06-26 at 07:53 +0200, Christoph Hellwig wrote:

quoted

On Tue, Jun 26, 2007 at 11:48:51AM +1000, Michael Ellerman wrote:

quoted

I realise jprobes are a razor-blades-included type of interface, but
that doesn't mean we can't try and make them safer to use. This guy I
know once wrote code like this:

struct jprobe jp = { .kp.symbol_name = "foo", .entry = "jprobe_foo" };

And then his kernel exploded. Oops.

This patch adds an arch hook, arch_deref_entry_point() (I don't like it either)
which takes the void * in a struct jprobe, and gives back the text address
that it represents.

We can then use that in register_jprobe() to check that the entry point
we're passed is actually in the kernel text, rather than just some random
value.

Please don't add more weak functions, they're utterly horrible for
anyone trying to understand the code.  Otherwise this looks fine to me.

What do you recommend instead? #define ARCH_HAS_FOO_BAR ?

o lord, save us, no.

I don't see what's utterly horrible about them.

Me either.

The fact that they're
weak is fairly reasonable documentation that they're overridden
somewhere else. And grep/cscope/ctags will find both the weak and
non-weak versions for you?

yup.

In this case we could just require that each jprobes-supporting
architecture implement arch_deref_entry_point().


Or one could do the Linus trick.  In each architecture which implements
arch_deref_entry_point() do:

#define arch_deref_entry_point arch_deref_entry_point

in the per-arch header file then, in non-arch code, do

#ifndef arch_deref_entry_point
static unsigned long arch_deref_entry_point(...)
{
	<generic implementation>
}
#endif

That's just the ARCH_HAS_FOO_BAR thing, only less fugly.