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

From: Jiri Olsa
Date: Tue Apr 14 2020 - 12:05:23 EST


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