Re: [PATCH v3] tracing: precise log info for kretprobe addr err

[Masami Hiramatsu <mhiramat@kernel.org>]

On Tue, 26 Jan 2021 16:40:38 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 26 Jan 2021 22:17:23 +0100
> Oleg Nesterov <oleg@redhat.com> wrote:
> 
> > On 01/26, Steven Rostedt wrote:
> > >
> > > On Tue, 26 Jan 2021 21:20:59 +0100
> > > Oleg Nesterov <oleg@redhat.com> wrote:
> > >  
> > > > > No, not wrong. Even offset != 0, if the symbol exists in the kernel,
> > > > > kprobe_on_func_entry() will check it.  
> > > >
> > > > Yes, but unless I am totally confused... if kprobe_on_func_entry() returns false,
> > > > then trace_kprobe_create() should fail with BAD_RETPROBE even if offset == 0 ?  
> > >
> > > From what I understand. kprobe_on_func_entry() can return false if you pass
> > > in: "MOD:not_yet_loaded_module_func", but this is OK, because when the
> > > module is loaded, and the "not_yet_loaded_module_func" exists, the
> > > kretprobe will then be added.
> > >
> > > The strchr(symbol,":") check is to see if "MOD:" (or some other ":" command)
> > > is in the name, and we don't want it to fail if it is. Which is why we
> > > should have that commented.  
> > 
> > Agreed, this matches my understanding.
> > 
> > But just in case... not sure I read this code correctly, but I think that
> > module_kallsyms_lookup_name("not_yet_loaded_module_func") should work even
> > without the "MOD:" prefix.
> > 
> > IOW, kprobe_on_func_entry("not_yet_loaded_module_func") can fail, and then
> > later succeed if you load the module which provides this symbol.
> > 
> > But even if I am right, I agree with the strchr(symbol,":") check.
> 
> I see what you are saying. If "MOD" is not loaded yet, the
> kprobe_on_func_entry() should succeed.

No, that makes me more confused. kprobes_on_func_entry() returns true
only if it confirms the given address is on the function entry, because
it is used in the register_kretprobe() too.

OK, I will make a separate check which detects an error that the
module is loaded but the symbol does not exist.
(current code doesn't check the module is loaded or not)

That makes the code clearer.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>