Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6076905b authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Alasdair G Kergon
Browse files

dm: avoid _hash_lock deadlock 



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)

Cc: stable@kernel.org
Reported-by: default avatarguy keren <choo@actcom.co.il>
Signed-off-by: default avatarMikulas Patocka <mpatocka@redhat.com>
Signed-off-by: default avatarAlasdair G Kergon <agk@redhat.com>
parent 3067e02f

drivers/md/dm-ioctl.c

+13 −4
Original line number Original line Diff line number Diff line
@@ -56,6 +56,11 @@ static void dm_hash_remove_all(int keep_open_devices); 
 */

 */

static DECLARE_RWSEM(_hash_lock);

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)

static void init_buckets(struct list_head *buckets)

{

{

	unsigned int i;

	unsigned int i;
@@ -206,7 +211,9 @@ static int dm_hash_insert(const char *name, const char *uuid, struct mapped_devi 
		list_add(&cell->uuid_list, _uuid_buckets + hash_str(uuid));

		list_add(&cell->uuid_list, _uuid_buckets + hash_str(uuid));

	}

	}

	dm_get(md);

	dm_get(md);

	mutex_lock(&dm_hash_cells_mutex);

	dm_set_mdptr(md, cell);

	dm_set_mdptr(md, cell);

	mutex_unlock(&dm_hash_cells_mutex);

	up_write(&_hash_lock);

	up_write(&_hash_lock);





	return 0;

	return 0;
@@ -224,7 +231,9 @@ static void __hash_remove(struct hash_cell *hc) 
	/* remove from the dev hash */

	/* remove from the dev hash */

	list_del(&hc->uuid_list);

	list_del(&hc->uuid_list);

	list_del(&hc->name_list);

	list_del(&hc->name_list);

	mutex_lock(&dm_hash_cells_mutex);

	dm_set_mdptr(hc->md, NULL);

	dm_set_mdptr(hc->md, NULL);

	mutex_unlock(&dm_hash_cells_mutex);





	table = dm_get_table(hc->md);

	table = dm_get_table(hc->md);

	if (table) {

	if (table) {
@@ -321,7 +330,9 @@ static int dm_hash_rename(uint32_t cookie, const char *old, const char *new) 
	 */

	 */

	list_del(&hc->name_list);

	list_del(&hc->name_list);

	old_name = hc->name;

	old_name = hc->name;

	mutex_lock(&dm_hash_cells_mutex);

	hc->name = new_name;

	hc->name = new_name;

	mutex_unlock(&dm_hash_cells_mutex);

	list_add(&hc->name_list, _name_buckets + hash_str(new_name));

	list_add(&hc->name_list, _name_buckets + hash_str(new_name));





	/*

	/*
@@ -1582,8 +1593,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) 
	if (!md)

	if (!md)

		return -ENXIO;

		return -ENXIO;





	dm_get(md);

	mutex_lock(&dm_hash_cells_mutex);

	down_read(&_hash_lock);

	hc = dm_get_mdptr(md);

	hc = dm_get_mdptr(md);

	if (!hc || hc->md != md) {

	if (!hc || hc->md != md) {

		r = -ENXIO;

		r = -ENXIO;
@@ -1596,8 +1606,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) 
		strcpy(uuid, hc->uuid ? : "");

		strcpy(uuid, hc->uuid ? : "");





out:

out:

	up_read(&_hash_lock);

	mutex_unlock(&dm_hash_cells_mutex);

	dm_put(md);





	return r;

	return r;

}
 
}

drivers/md/dm-uevent.c

+4 −5
Original line number Original line Diff line number Diff line
@@ -139,13 +139,12 @@ void dm_send_uevents(struct list_head *events, struct kobject *kobj) 
		list_del_init(&event->elist);

		list_del_init(&event->elist);





		/*

		/*

		 * Need to call dm_copy_name_and_uuid from here for now.

		 * When a device is being removed this copy fails and we

		 * Context of previous var adds and locking used for

		 * discard these unsent events.

		 * hash_cell not compatable.

		 */

		 */

		if (dm_copy_name_and_uuid(event->md, event->name,

		if (dm_copy_name_and_uuid(event->md, event->name,

					  event->uuid)) {

					  event->uuid)) {

			DMERR("%s: dm_copy_name_and_uuid() failed",

			DMINFO("%s: skipping sending uevent for lost device",

			       __func__);

			       __func__);

			goto uevent_free;

			goto uevent_free;

		}

		}