[PATCH] [SCSI] scsi_debug: fix deadlock on module unload, also complete queued commands

[PATCH] [SCSI] scsi_debug: fix deadlock on module unload, also complete queued commands

[Ewan D. Milne]

From: "Ewan D. Milne" <emilne@redhat.com>

The module unload path in scsi_debug was calling del_timer_sync() with the
queued_arr_lock held.  The timer_intr_handler() function also takes this lock,
so if it executes as the timer is being removed, the kernel would deadlock.

Also changed to complete all queued commands if scsi_debug is unloaded instead
of discarding them, because the requests would eventually timeout after the
module was gone.

Signed-off-by: Ewan D. Milne <emilne@redhat.com>
---
 drivers/scsi/scsi_debug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..f10c1a9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2483,8 +2483,15 @@ static int stop_queued_cmnd(struct scsi_cmnd *cmnd)
 	for (k = 0; k < scsi_debug_max_queue; ++k) {
 		sqcp = &queued_arr[k];
 		if (sqcp->in_use && (cmnd == sqcp->a_cmnd)) {
+			spin_unlock_irqrestore(&queued_arr_lock, iflags);
 			del_timer_sync(&sqcp->cmnd_timer);
+			spin_lock_irqsave(&queued_arr_lock, iflags);
 			sqcp->in_use = 0;
+			if (sqcp->done_funct) {
+				sqcp->a_cmnd->result = sqcp->scsi_result;
+				sqcp->done_funct(sqcp->a_cmnd);
+			}
+			sqcp->done_funct = NULL;
 			sqcp->a_cmnd = NULL;
 			break;
 		}
@@ -2504,8 +2511,15 @@ static void stop_all_queued(void)
 	for (k = 0; k < scsi_debug_max_queue; ++k) {
 		sqcp = &queued_arr[k];
 		if (sqcp->in_use && sqcp->a_cmnd) {
+			spin_unlock_irqrestore(&queued_arr_lock, iflags);
 			del_timer_sync(&sqcp->cmnd_timer);
+			spin_lock_irqsave(&queued_arr_lock, iflags);
 			sqcp->in_use = 0;
+			if (sqcp->done_funct) {
+				sqcp->a_cmnd->result = sqcp->scsi_result;
+				sqcp->done_funct(sqcp->a_cmnd);
+			}
+			sqcp->done_funct = NULL;
 			sqcp->a_cmnd = NULL;
 		}
 	}
-- 
1.7.11.7

Re: [PATCH] [SCSI] scsi_debug: fix deadlock on module unload, also complete queued commands

[Douglas Gilbert]

On 12-12-13 04:00 AM, Ewan D. Milne wrote:
> From: "Ewan D. Milne" <emilne@redhat.com>
>
> The module unload path in scsi_debug was calling del_timer_sync() with the
> queued_arr_lock held.  The timer_intr_handler() function also takes this lock,
> so if it executes as the timer is being removed, the kernel would deadlock.
>
> Also changed to complete all queued commands if scsi_debug is unloaded instead
> of discarding them, because the requests would eventually timeout after the
> module was gone.
>
> Signed-off-by: Ewan D. Milne <emilne@redhat.com>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>