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

[Jiri Olsa]

On Fri, Apr 10, 2020 at 09:31:07AM +0800, Ziqian SUN (Zamir) wrote:
> 
> 
> On 4/10/20 4:13 AM, Jiri Olsa wrote:
> > On Thu, Apr 09, 2020 at 08:45:01PM +0200, Jiri Olsa wrote:
> > > 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
> > 
> > ok, found out it's actually arch code..  would you guys be ok with something like below?
> > 
> > jirka
> > 
> 
> Hi Jiri,
> 
> In my origin test lockup happens on both x86_64 and ppc64le. So I would
> appreciate if you can also come up with a solution for both of the
> architectures.

aaaah right.. will update the fix

thanks,
jirka