| 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
| ||
|
Date: Wed, 16 Dec 2009 19:56:40 -0800
From: Greg KH <gregkh@...e.de>
To: linux-kernel@...r.kernel.org, stable@...nel.org
Cc: stable-review@...nel.org, torvalds@...ux-foundation.org,
akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
Mikulas Patocka <mpatocka@...hat.com>,
Alasdair G Kergon <agk@...hat.com>
Subject: [103/151] dm: avoid _hash_lock deadlock
2.6.32-stable review patch. If anyone has any objections, please let us know.
------------------
From: Mikulas Patocka <mpatocka@...hat.com>
commit 6076905b5ef39e0ea58db32583c9e0036c05e47b upstream.
Fix a reported deadlock if there are still unprocessed multipath events
on a device that is being removed.
_hash_lock is held during dev_remove while trying to send the
outstanding events. Sending the events requests the _hash_lock
again in dm_copy_name_and_uuid.
This patch introduces a separate lock around regions that modify the
link to the hash table (dm_set_mdptr) or the name or uuid so that
dm_copy_name_and_uuid no longer needs _hash_lock.
Additionally, dm_copy_name_and_uuid can only be called if md exists
so we can drop the dm_get() and dm_put() which can lead to a BUG()
while md is being freed.
The deadlock:
#0 [ffff8106298dfb48] schedule at ffffffff80063035
#1 [ffff8106298dfc20] __down_read at ffffffff8006475d
#2 [ffff8106298dfc60] dm_copy_name_and_uuid at ffffffff8824f740
#3 [ffff8106298dfc90] dm_send_uevents at ffffffff88252685
#4 [ffff8106298dfcd0] event_callback at ffffffff8824c678
#5 [ffff8106298dfd00] dm_table_event at ffffffff8824dd01
#6 [ffff8106298dfd10] __hash_remove at ffffffff882507ad
#7 [ffff8106298dfd30] dev_remove at ffffffff88250865
#8 [ffff8106298dfd60] ctl_ioctl at ffffffff88250d80
#9 [ffff8106298dfee0] do_ioctl at ffffffff800418c4
#10 [ffff8106298dff00] vfs_ioctl at ffffffff8002fab9
#11 [ffff8106298dff40] sys_ioctl at ffffffff8004bdaf
#12 [ffff8106298dff80] tracesys at ffffffff8005d28d (via system_call)
Reported-by: guy keren <choo@...com.co.il>
Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
Signed-off-by: Alasdair G Kergon <agk@...hat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>
---
drivers/md/dm-ioctl.c | 17 +++++++++++++----
drivers/md/dm-uevent.c | 9 ++++-----
2 files changed, 17 insertions(+), 9 deletions(-)
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -56,6 +56,11 @@ static void dm_hash_remove_all(int keep_
*/
static DECLARE_RWSEM(_hash_lock);
+/*
+ * Protects use of mdptr to obtain hash cell name and uuid from mapped device.
+ */
+static DEFINE_MUTEX(dm_hash_cells_mutex);
+
static void init_buckets(struct list_head *buckets)
{
unsigned int i;
@@ -206,7 +211,9 @@ static int dm_hash_insert(const char *na
list_add(&cell->uuid_list, _uuid_buckets + hash_str(uuid));
}
dm_get(md);
+ mutex_lock(&dm_hash_cells_mutex);
dm_set_mdptr(md, cell);
+ mutex_unlock(&dm_hash_cells_mutex);
up_write(&_hash_lock);
return 0;
@@ -224,7 +231,9 @@ static void __hash_remove(struct hash_ce
/* remove from the dev hash */
list_del(&hc->uuid_list);
list_del(&hc->name_list);
+ mutex_lock(&dm_hash_cells_mutex);
dm_set_mdptr(hc->md, NULL);
+ mutex_unlock(&dm_hash_cells_mutex);
table = dm_get_table(hc->md);
if (table) {
@@ -321,7 +330,9 @@ static int dm_hash_rename(uint32_t cooki
*/
list_del(&hc->name_list);
old_name = hc->name;
+ mutex_lock(&dm_hash_cells_mutex);
hc->name = new_name;
+ mutex_unlock(&dm_hash_cells_mutex);
list_add(&hc->name_list, _name_buckets + hash_str(new_name));
/*
@@ -1582,8 +1593,7 @@ int dm_copy_name_and_uuid(struct mapped_
if (!md)
return -ENXIO;
- dm_get(md);
- down_read(&_hash_lock);
+ mutex_lock(&dm_hash_cells_mutex);
hc = dm_get_mdptr(md);
if (!hc || hc->md != md) {
r = -ENXIO;
@@ -1596,8 +1606,7 @@ int dm_copy_name_and_uuid(struct mapped_
strcpy(uuid, hc->uuid ? : "");
out:
- up_read(&_hash_lock);
- dm_put(md);
+ mutex_unlock(&dm_hash_cells_mutex);
return r;
}
--- a/drivers/md/dm-uevent.c
+++ b/drivers/md/dm-uevent.c
@@ -139,14 +139,13 @@ void dm_send_uevents(struct list_head *e
list_del_init(&event->elist);
/*
- * Need to call dm_copy_name_and_uuid from here for now.
- * Context of previous var adds and locking used for
- * hash_cell not compatable.
+ * When a device is being removed this copy fails and we
+ * discard these unsent events.
*/
if (dm_copy_name_and_uuid(event->md, event->name,
event->uuid)) {
- DMERR("%s: dm_copy_name_and_uuid() failed",
- __func__);
+ DMINFO("%s: skipping sending uevent for lost device",
+ __func__);
goto uevent_free;
}
--
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/
Powered by blists - more mailing lists