Re: [PATCH] jprobes: Ensure that the probepoint is at function entry

[Masami Hiramatsu <mhiramat@kernel.org>]

On Thu, 6 Jul 2017 14:15:49 +0200
Ingo Molnar <mingo@kernel.org> wrote:

> * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > Also, 'function_offset_within_entry' is way too long a name, and it's also a 
> > > minomer I think. The purpose of this function is to enforce that the relative 
> > > 'offset' of a new probe is at the standard function entry offset: i.e. 0 on most 
> > > architectures, and some ABI dependent constant on PowerPC, right?
> > > 
> > > That's not at all clear from that name, plus it's a global namespace symbol, yet 
> > > has no 'kprobes' prefix. So it should be named something like 
> > > 'kprobe_offset_valid()' or such, with an arch_kprobe_offset_valid() counterpart.
> > 
> > Hmm, I would rather like kprobe_within_entry(), since offset != 0 is
> > actually valid for normal kprobe, that is kretprobe and jprobe limitation.
> 
> But what entry? That it's within a range or that offset is always 0 is really an 
> implementational detail: depending on what type of kprobe it is, it is either 
> validly within the confines of the specified function symbol or not.

Hmm, right. In most cases, it just checks the address (symbol+offset) is
on the function entry.

> What _really_ matters to callers is whether it's a valid kprobe to be inserted 
> into that function, right?

No, for that purpose, kprobes checks it in other places (kprobe_addr() and check_kprobe_address_safe()). This function is an additional safety check
only for kretprobe and jprobe which must be placed on the function entry.
(kprobe can probe function body but kretprobe and jprobes are not)
 
> I.e. the long name came from over-specifying what is done by the function - while 
> simplifying makes it actually more meaningful to read.

I see, but kprobe_offset_valid is too simple. How about kprobe_on_func_entry()?

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>