Re: [PATCH 2/5] kprobes: Use guard() for external locks

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Tue, 10 Dec 2024 13:10:27 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> 
> > Wait, this is for checking the jump_label_text_reserved(), but as far as
> > I know, the text reserved area of jump_label will be updated when the
> > module is loaded or removed. And the static call too, right?
> 
> Correct.
> 
> > In that case, what we need is to lock the modules (for the short term,
> > can we use rcu_read_lock?) for using both jump_label_text_reserved()
> > and static_call_text_reserved()?
> 
> Yes, rcu_read_lock() is sufficient to observe fully loaded modules. I
> don't think you care about placing kprobes on modules that are still
> loading (that doesn't really make sense).

Actually, to probe module's __init function, it may put a probe during
loading modules (by trace_kprobe.c) which has been done by module
notification callback.

trace_kprobe_module_callback()
 -> register_module_trace_kprobe()
   -> __register_trace_kprobe()
      -> register_kprobe()
         -> check_kprobe_address_safe()

Anyway, unless we run the module notifier callbacks in parallel,
it should be safe.

Hmm, however, it seems that trace_probe's module notifier priority
is not correct. It must be lower than jump_label but it is the same.

OK, let me remove jump_label_lock() from kprobes (if it gets
module reference), and give a lower priority to the trace_probe's
module notifier to ensure it is called after jump_label is updated.

> 
> Also see:
> 
>   https://lkml.kernel.org/r/20241205215102.hRywUW2A@linutronix.de

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>