Commit 5e301eba authored by Shiva Krishna Merla's avatar Shiva Krishna Merla Committed by Greg Kroah-Hartman

dm mpath: fix race condition between multipath_dtr and pg_init_done

commit 954a73d5 upstream.

Whenever multipath_dtr() is happening we must prevent queueing any
further path activation work.  Implement this by adding a new
'pg_init_disabled' flag to the multipath structure that denotes future
path activation work should be skipped if it is set.  By disabling
pg_init and then re-enabling in flush_multipath_work() we also avoid the
potential for pg_init to be initiated while suspending an mpath device.

Without this patch a race condition exists that may result in a kernel
panic:

1) If after pg_init_done() decrements pg_init_in_progress to 0, a call
   to wait_for_pg_init_completion() assumes there are no more pending path
   management commands.
2) If pg_init_required is set by pg_init_done(), due to retryable
   mode_select errors, then process_queued_ios() will again queue the
   path activation work.
3) If free_multipath() completes before activate_path() work is called a
   NULL pointer dereference like the following can be seen when
   accessing members of the recently destructed multipath:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000090
RIP: 0010:[<ffffffffa003db1b>]  [<ffffffffa003db1b>] activate_path+0x1b/0x30 [dm_multipath]
[<ffffffff81090ac0>] worker_thread+0x170/0x2a0
[<ffffffff81096c80>] ? autoremove_wake_function+0x0/0x40

[switch to disabling pg_init in flush_multipath_work & header edits by Mike Snitzer]
Signed-off-by: default avatarShiva Krishna Merla <shivakrishna.merla@netapp.com>
Reviewed-by: default avatarKrishnasamy Somasundaram <somasundaram.krishnasamy@netapp.com>
Tested-by: default avatarSpeagle Andy <Andy.Speagle@netapp.com>
Acked-by: default avatarJunichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
[bwh: Backported to 3.2:
 - Adjust context
 - Bump version to 1.3.2 not 1.6.0]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
[xr: Backported to 3.4: Adjust context]
Signed-off-by: default avatarRui Xiang <rui.xiang@huawei.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent d110fd51
...@@ -84,6 +84,7 @@ struct multipath { ...@@ -84,6 +84,7 @@ struct multipath {
unsigned queue_io; /* Must we queue all I/O? */ unsigned queue_io; /* Must we queue all I/O? */
unsigned queue_if_no_path; /* Queue I/O if last path fails? */ unsigned queue_if_no_path; /* Queue I/O if last path fails? */
unsigned saved_queue_if_no_path;/* Saved state during suspension */ unsigned saved_queue_if_no_path;/* Saved state during suspension */
unsigned pg_init_disabled:1; /* pg_init is not currently allowed */
unsigned pg_init_retries; /* Number of times to retry pg_init */ unsigned pg_init_retries; /* Number of times to retry pg_init */
unsigned pg_init_count; /* Number of times pg_init called */ unsigned pg_init_count; /* Number of times pg_init called */
unsigned pg_init_delay_msecs; /* Number of msecs before pg_init retry */ unsigned pg_init_delay_msecs; /* Number of msecs before pg_init retry */
...@@ -493,7 +494,8 @@ static void process_queued_ios(struct work_struct *work) ...@@ -493,7 +494,8 @@ static void process_queued_ios(struct work_struct *work)
(!pgpath && !m->queue_if_no_path)) (!pgpath && !m->queue_if_no_path))
must_queue = 0; must_queue = 0;
if (m->pg_init_required && !m->pg_init_in_progress && pgpath) if (m->pg_init_required && !m->pg_init_in_progress && pgpath &&
!m->pg_init_disabled)
__pg_init_all_paths(m); __pg_init_all_paths(m);
out: out:
...@@ -907,10 +909,20 @@ static void multipath_wait_for_pg_init_completion(struct multipath *m) ...@@ -907,10 +909,20 @@ static void multipath_wait_for_pg_init_completion(struct multipath *m)
static void flush_multipath_work(struct multipath *m) static void flush_multipath_work(struct multipath *m)
{ {
unsigned long flags;
spin_lock_irqsave(&m->lock, flags);
m->pg_init_disabled = 1;
spin_unlock_irqrestore(&m->lock, flags);
flush_workqueue(kmpath_handlerd); flush_workqueue(kmpath_handlerd);
multipath_wait_for_pg_init_completion(m); multipath_wait_for_pg_init_completion(m);
flush_workqueue(kmultipathd); flush_workqueue(kmultipathd);
flush_work_sync(&m->trigger_event); flush_work_sync(&m->trigger_event);
spin_lock_irqsave(&m->lock, flags);
m->pg_init_disabled = 0;
spin_unlock_irqrestore(&m->lock, flags);
} }
static void multipath_dtr(struct dm_target *ti) static void multipath_dtr(struct dm_target *ti)
...@@ -1129,7 +1141,7 @@ static int pg_init_limit_reached(struct multipath *m, struct pgpath *pgpath) ...@@ -1129,7 +1141,7 @@ static int pg_init_limit_reached(struct multipath *m, struct pgpath *pgpath)
spin_lock_irqsave(&m->lock, flags); spin_lock_irqsave(&m->lock, flags);
if (m->pg_init_count <= m->pg_init_retries) if (m->pg_init_count <= m->pg_init_retries && !m->pg_init_disabled)
m->pg_init_required = 1; m->pg_init_required = 1;
else else
limit_reached = 1; limit_reached = 1;
...@@ -1644,7 +1656,7 @@ static int multipath_busy(struct dm_target *ti) ...@@ -1644,7 +1656,7 @@ static int multipath_busy(struct dm_target *ti)
*---------------------------------------------------------------*/ *---------------------------------------------------------------*/
static struct target_type multipath_target = { static struct target_type multipath_target = {
.name = "multipath", .name = "multipath",
.version = {1, 3, 0}, .version = {1, 3, 2},
.module = THIS_MODULE, .module = THIS_MODULE,
.ctr = multipath_ctr, .ctr = multipath_ctr,
.dtr = multipath_dtr, .dtr = multipath_dtr,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment