linux-kernel - Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1287202866.16971.47.camel@gandalf.stny.rr.com>
Date:	Sat, 16 Oct 2010 00:21:06 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Jason Baron <jbaron@...hat.com>
Subject: Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs.
 text_mutex

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);
>  
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/