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

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

On Tue, 10 Dec 2024 11:04:28 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Mon, 9 Dec 2024 12:04:11 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Mon, Dec 09, 2024 at 11:41:26AM +0900, Masami Hiramatsu (Google) wrote:
> > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > 
> > > Use guard() for text_mutex, cpu_read_lock, and jump_label_lock in
> > > the kprobes.
> > 
> > > @@ -853,29 +850,24 @@ static void try_to_optimize_kprobe(struct kprobe *p)
> > >  		return;
> > >  
> > >  	/* For preparing optimization, jump_label_text_reserved() is called. */
> > > -	cpus_read_lock();
> > > -	jump_label_lock();
> > > -	mutex_lock(&text_mutex);
> > > +	guard(cpus_read_lock)();
> > > +	guard(jump_label_lock)();
> > > +	guard(mutex)(&text_mutex);
> > >  
> > 
> > > @@ -1294,62 +1280,55 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
> > >  	int ret = 0;
> > >  	struct kprobe *ap = orig_p;
> > >  
> > > -	cpus_read_lock();
> > > -
> > > -	/* For preparing optimization, jump_label_text_reserved() is called */
> > > -	jump_label_lock();
> > > -	mutex_lock(&text_mutex);
> > 
> > Why does kprobe need jump_label_lock and how does it then not also need
> > static_call_lock ?
> 
> Good catch! It has not been updated for static_call_text_reserved().
> We need static_call_lock() here too.

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?

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()?

Thank you,

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