lkml.org 
[lkml]   [2010]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex
From
Date
On Fri, 2010-10-15 at 22:25 -0400, Steven Rostedt wrote:
> On Fri, 2010-10-15 at 22:55 +0200, Peter Zijlstra wrote:
> > On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > > +void jump_label_lock(void)
> > > +{
> > > + mutex_lock(&jump_label_mutex);
> > > +}
> > > +
> >
> > > +++ b/kernel/kprobes.c
> > > @@ -1145,13 +1145,16 @@ int __kprobes register_kprobe(struct kprobe *p)
> > > return ret;
> > >
> > > preempt_disable();
> > > + jump_label_lock();
> >
> > How exactly does that work?
> >
>
> This patch should fix it (compiled tested only). If all agree, I'll add
> it to the series.

Well, I just did more than compile test it. I booted first without this
patch, and added a kprobe and lockdep spewed its guts about this
condition. Then I applied the patch, compiled and rebooted, tried the
probe again, and lockdep stayed quiet :-)

-- Steve


> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d45d72e..0dbd328 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1144,17 +1144,13 @@ int __kprobes register_kprobe(struct kprobe *p)
> if (ret)
> return ret;
>
> - preempt_disable();
> jump_label_lock();
> + preempt_disable();
> if (!kernel_text_address((unsigned long) p->addr) ||
> in_kprobes_functions((unsigned long) p->addr) ||
> ftrace_text_reserved(p->addr, p->addr) ||
> - jump_label_text_reserved(p->addr, p->addr)) {
> - preempt_enable();
> - jump_label_unlock();
> - return -EINVAL;
> - }
> - jump_label_unlock();
> + jump_label_text_reserved(p->addr, p->addr))
> + goto fail_with_jump_label;
>
> /* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */
> p->flags &= KPROBE_FLAG_DISABLED;
> @@ -1168,10 +1164,9 @@ int __kprobes register_kprobe(struct kprobe *p)
> * We must hold a refcount of the probed module while updating
> * its code to prohibit unexpected unloading.
> */
> - if (unlikely(!try_module_get(probed_mod))) {
> - preempt_enable();
> - return -EINVAL;
> - }
> + if (unlikely(!try_module_get(probed_mod)))
> + goto fail_with_jump_label;
> +
> /*
> * If the module freed .init.text, we couldn't insert
> * kprobes in there.
> @@ -1179,11 +1174,11 @@ int __kprobes register_kprobe(struct kprobe *p)
> if (within_module_init((unsigned long)p->addr, probed_mod) &&
> probed_mod->state != MODULE_STATE_COMING) {
> module_put(probed_mod);
> - preempt_enable();
> - return -EINVAL;
> + goto fail_with_jump_label;
> }
> }
> preempt_enable();
> + jump_label_unlock();
>
> p->nmissed = 0;
> INIT_LIST_HEAD(&p->list);
> @@ -1225,6 +1220,11 @@ out:
> module_put(probed_mod);
>
> return ret;
> +
> +fail_with_jump_label:
> + preempt_enable();
> + jump_label_unlock();
> + return -EINVAL;
> }
> EXPORT_SYMBOL_GPL(register_kprobe);
>
>




\
 
 \ /
  Last update: 2010-10-16 06:23    [W:0.078 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site