linux-kernel - Re: [PATCH 01/51] CPU hotplug: Provide lockless versions of callback registration functions

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: <20140207191125.GA8098@in.ibm.com>
Date:	Sat, 8 Feb 2014 00:41:25 +0530
From:	Gautham R Shenoy <ego@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	paulus@...ba.org, rusty@...tcorp.com.au, peterz@...radead.org,
	tglx@...utronix.de, akpm@...ux-foundation.org, mingo@...nel.org,
	paulmck@...ux.vnet.ibm.com, tj@...nel.org, walken@...gle.com,
	ego@...ux.vnet.ibm.com, linux@....linux.org.uk,
	linux-kernel@...r.kernel.org, Toshi Kani <toshi.kani@...com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH 01/51] CPU hotplug: Provide lockless versions of
	callback registration functions

On Thu, Feb 06, 2014 at 07:41:03PM +0100, Oleg Nesterov wrote:
> On 02/06, Srivatsa S. Bhat wrote:
> >
> > The following method of CPU hotplug callback registration is not safe
> > due to the possibility of an ABBA deadlock involving the cpu_add_remove_lock
> > and the cpu_hotplug.lock.
> 
> Off-topic, but perhaps it also makes sense to add the lockdep annotations
> later, to catch other similar problems. Currently get_online_cpus() acquires
> nothing from lockdep pov.

Well, both get/put_online_cpus() as well as cpu_hotplug_begin/end()
take the cpu_hotplug.lock mutex. So ideally the lockdep annotations of
mutex_lock/unlock() should have worked. If it hasn't, then the
following lockdep annotations to cpu-hotplug locking should do the
trick.

Signed-off-by: Gautham R. Shenoy <ego@...ux.vnet.ibm.com>
---
 kernel/cpu.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index deff2e6..3d2dd1c 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -19,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/gfp.h>
 #include <linux/suspend.h>
+#include <linux/lockdep.h>
 
 #include "smpboot.h"
 
@@ -57,21 +58,34 @@ static struct {
 	 * an ongoing cpu hotplug operation.
 	 */
 	int refcount;
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct lockdep_map dep_map;
+#endif
 } cpu_hotplug = {
 	.active_writer = NULL,
 	.lock = __MUTEX_INITIALIZER(cpu_hotplug.lock),
 	.refcount = 0,
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	.dep_map = {.name = "cpu_hotplug.lock" },
+#endif
 };
 
+#define cphp_lock_acquire_read(l, s, t, i) lock_acquire_shared_recursive(l, s, t, NULL, i)
+#define cphp_lock_acquire(l, s, t, i)      lock_acquire_exclusive(l, s, t, NULL, i)
+#define cphp_lock_release(l, n, i)      lock_release(l, n, i)
+
 void get_online_cpus(void)
 {
 	might_sleep();
 	if (cpu_hotplug.active_writer == current)
 		return;
+	cphp_lock_acquire_read(&cpu_hotplug.dep_map, 0, 0, _RET_IP_);
 	mutex_lock(&cpu_hotplug.lock);
 	cpu_hotplug.refcount++;
 	mutex_unlock(&cpu_hotplug.lock);
 
+
 }
 EXPORT_SYMBOL_GPL(get_online_cpus);
 
@@ -79,6 +93,7 @@ void put_online_cpus(void)
 {
 	if (cpu_hotplug.active_writer == current)
 		return;
+
 	mutex_lock(&cpu_hotplug.lock);
 
 	if (WARN_ON(!cpu_hotplug.refcount))
@@ -87,6 +102,7 @@ void put_online_cpus(void)
 	if (!--cpu_hotplug.refcount && unlikely(cpu_hotplug.active_writer))
 		wake_up_process(cpu_hotplug.active_writer);
 	mutex_unlock(&cpu_hotplug.lock);
+	cphp_lock_release(&cpu_hotplug.dep_map, 1, _RET_IP_);
 
 }
 EXPORT_SYMBOL_GPL(put_online_cpus);
@@ -117,6 +133,7 @@ void cpu_hotplug_begin(void)
 {
 	cpu_hotplug.active_writer = current;
 
+	cphp_lock_acquire(&cpu_hotplug.dep_map, 0, 0, _RET_IP_);
 	for (;;) {
 		mutex_lock(&cpu_hotplug.lock);
 		if (likely(!cpu_hotplug.refcount))
@@ -131,6 +148,7 @@ void cpu_hotplug_done(void)
 {
 	cpu_hotplug.active_writer = NULL;
 	mutex_unlock(&cpu_hotplug.lock);
+	cphp_lock_release(&cpu_hotplug.dep_map, 1, _RET_IP_);
 }
 
 /*
-- 
1.8.3.1

--
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/