linux-kernel - Re: [PATCH] slub: Fix sysfs circular locking dependency

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]

Date:	Fri, 7 Jan 2011 09:22:46 -0600 (CST)
From:	Christoph Lameter <cl@...ux.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
cc:	David Rientjes <rientjes@...gle.com>,
	Pekka Enberg <penberg@...nel.org>,
	Bart Van Assche <bvanassche@....org>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] slub: Fix sysfs circular locking dependency

On Fri, 7 Jan 2011, KAMEZAWA Hiroyuki wrote:

> But, nice catch. I think MEM_GOING_ONLINE, MEM_ONLINE should be done under
> lock_memory_hotplug. So, could you update your patch and modify online_pages() ?
>
> IIUC, online_pages() is an user interface function and there will be no downside
> to insert lock there. online_pages() should be serialized.

Subject: slub: Avoid use of slub_lock in show_slab_objects()

The purpose of the locking is to prevent removal and additions
of nodes when statistics are gathered for a slab cache. So we
need to avoid racing with memory hotplug functionality.

It is enough to take the memory hotplug locks there instead
of the slub_lock.

online_pages() does not acquire the memory_hotplug lock. So
add the missing locking there.

Signed-off-by: Christoph Lameter <cl@...ux.com>

---
 mm/memory_hotplug.c |   17 +++++++++++------
 mm/slub.c           |    4 ++--
 2 files changed, 13 insertions(+), 8 deletions(-)

Index: linux-2.6/mm/slub.c
===================================================================
--- linux-2.6.orig/mm/slub.c	2011-01-07 09:17:35.000000000 -0600
+++ linux-2.6/mm/slub.c	2011-01-07 09:17:38.000000000 -0600
@@ -3821,7 +3821,7 @@ static ssize_t show_slab_objects(struct
 		}
 	}

-	down_read(&slub_lock);
+	lock_memory_hotplug();
 #ifdef CONFIG_SLUB_DEBUG
 	if (flags & SO_ALL) {
 		for_each_node_state(node, N_NORMAL_MEMORY) {
@@ -3862,7 +3862,7 @@ static ssize_t show_slab_objects(struct
 			x += sprintf(buf + x, " N%d=%lu",
 					node, nodes[node]);
 #endif
-	up_read(&slub_lock);
+	unlock_memory_hotplug();
 	kfree(nodes);
 	return x + sprintf(buf + x, "\n");
 }
Index: linux-2.6/mm/memory_hotplug.c
===================================================================
--- linux-2.6.orig/mm/memory_hotplug.c	2011-01-07 09:17:59.000000000 -0600
+++ linux-2.6/mm/memory_hotplug.c	2011-01-07 09:20:07.000000000 -0600
@@ -415,12 +415,12 @@ int online_pages(unsigned long pfn, unsi
 	if (node_present_pages(nid) == 0)
 		arg.status_change_nid = nid;

+	lock_memory_hotplug();
 	ret = memory_notify(MEM_GOING_ONLINE, &arg);
 	ret = notifier_to_errno(ret);
-	if (ret) {
-		memory_notify(MEM_CANCEL_ONLINE, &arg);
-		return ret;
-	}
+	if (ret)
+		goto cancel;
+
 	/*
 	 * This doesn't need a lock to do pfn_to_page().
 	 * The section can't be removed here because of the
@@ -442,8 +442,7 @@ int online_pages(unsigned long pfn, unsi
 		mutex_unlock(&zonelists_mutex);
 		printk(KERN_DEBUG "online_pages %lx at %lx failed\n",
 			nr_pages, pfn);
-		memory_notify(MEM_CANCEL_ONLINE, &arg);
-		return ret;
+		goto cancel;
 	}

 	zone->present_pages += onlined_pages;
@@ -468,7 +467,13 @@ int online_pages(unsigned long pfn, unsi
 	if (onlined_pages)
 		memory_notify(MEM_ONLINE, &arg);

+	unlock_memory_hotplug();
 	return 0;
+
+cancel:
+	memory_notify(MEM_CANCEL_ONLINE, &arg);
+	unlock_memory_hotplug();
+	return ret;
 }
 #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */

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