On Tue, 10 Dec 2024 23:12:41 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

On Tue, 10 Dec 2024 13:10:27 +0100
Peter Zijlstra [off-list ref] wrote:

quoted

quoted

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.

quoted

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, this is still a problem. We need to acquire jump_label_lock()
even for the jump_label_text_reserved().

If user runs module load and register_kprobe() in parallel, 
jump_label_module_notify() and check_kprobe_address_safe() may run
in parallel.

jump_label_module_notify()
  -> jump_label_add_module()
     -> jump_label_sort_entries() <- update mod->jump_entries

check_kprobe_address_safe()
  -> jump_label_text_reserved()
      -> __jump_label_mod_text_reserved() <- read mod->jump_entries

So there is a race on mod->jump_entries. jump_label_lock() avoids
this race.
(IIRC, module can get the reference in the MODULE_STATE_COMING state.)

On the other hand, static_call_text_reserved() does not need a lock
because it does not sort the list, nor update the static_call_site::addr.

In conclusion, we need jump_label_lock() as it is, and don't need
static_call_lock().

Thank you,

-- 
Masami Hiramatsu (Google) [off-list ref]