Re: [RFC] kretprobe: Prevent triggering kretprobe from within kprobe_flush_task

[Jiri Olsa]

On Thu, Apr 09, 2020 at 11:41:01PM +0900, Masami Hiramatsu wrote:

SNIP

> > ---
> >  kernel/kprobes.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 2625c241ac00..b13247cae752 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1236,6 +1236,10 @@ __releases(hlist_lock)
> >  }
> >  NOKPROBE_SYMBOL(kretprobe_table_unlock);
> >  
> > +static struct kprobe kretprobe_dummy = {
> > +        .addr = (void *)kretprobe_trampoline,
> > +};
> > +
> >  /*
> >   * This function is called from finish_task_switch when task tk becomes dead,
> >   * so that we can recycle any function-return probe instances associated
> > @@ -1256,12 +1260,14 @@ void kprobe_flush_task(struct task_struct *tk)
> >  	INIT_HLIST_HEAD(&empty_rp);
> >  	hash = hash_ptr(tk, KPROBE_HASH_BITS);
> >  	head = &kretprobe_inst_table[hash];
> > +	__this_cpu_write(current_kprobe, &kretprobe_dummy);
> 
> Can you also set the kcb->kprobe_state = KPROBE_HIT_ACTIVE?
> 
> BTW, we may be better to introduce a common kprobe_reject_section_start()
> and kprobe_reject_section_end() so that the user don't need to prepare
> dummy kprobes.

sure, will do

thank you both for review
jirka

> 
> Thank you,
> 
> >  	kretprobe_table_lock(hash, &flags);
> >  	hlist_for_each_entry_safe(ri, tmp, head, hlist) {
> >  		if (ri->task == tk)
> >  			recycle_rp_inst(ri, &empty_rp);
> >  	}
> >  	kretprobe_table_unlock(hash, &flags);
> > +	__this_cpu_write(current_kprobe, NULL);
> >  	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
> >  		hlist_del(&ri->hlist);
> >  		kfree(ri);
> > -- 
> > 2.25.2
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>