From 67c99a72e3006e4276e91d7282a3d6734fc77a0b Mon Sep 17 00:00:00 2001 From: "scameron@beardog.cce.hp.com" Date: Mon, 14 Apr 2014 14:01:09 -0500 Subject: [PATCH 1/7] [SCSI] hpsa: fix NULL dereference in hpsa_put_ctlr_into_performant_mode() Initialize local variable trans_support before it is used rather than after. It is supposed to contain the value of a register on the controller containing bits that describe which transport modes the controller supports (e.g. "performant", "ioaccel1", "ioaccel2"). A NULL pointer dereference will almost certainly occur if trans_support is not initialized at the right point. If for example the uninitialized trans_support value does not have the bit set for ioaccel2 support when it should be, then ioaccel2_alloc_cmds_and_bft() will not get called as it should be and the h->ioaccel2_blockFetchTable array will remain NULL instead of being allocated. Too late, trans_support finally gets initialized with the correct value with ioaccel2 mode bit set, which later causes calc_bucket_map() to be called to fill in h->ioaccel2_blockFetchTable[]. However h->ioaccel2_blockFetchTable is NULL because it didn't get allocated because earlier trans_support wasn't initialized at the right point. Fixes: e1f7de0cdd68d246d7008241cd9e443a54f880a8 Signed-off-by: Stephen M. Cameron Reported-by: Baoquan He Tested-by: Baoquan He Signed-off-by: James Bottomley --- drivers/scsi/hpsa.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 8cf4a0c69baf..9a6e4a2cd072 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h) if (hpsa_simple_mode) return; + trans_support = readl(&(h->cfgtable->TransportSupport)); + if (!(trans_support & PERFORMANT_MODE)) + return; + /* Check for I/O accelerator mode support */ if (trans_support & CFGTBL_Trans_io_accel1) { transMethod |= CFGTBL_Trans_io_accel1 | @@ -7479,10 +7483,6 @@ static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h) } /* TODO, check that this next line h->nreply_queues is correct */ - trans_support = readl(&(h->cfgtable->TransportSupport)); - if (!(trans_support & PERFORMANT_MODE)) - return; - h->nreply_queues = h->msix_vector > 0 ? h->msix_vector : 1; hpsa_get_max_perf_mode_cmds(h); /* Performant mode ring buffer and supporting data structures */ From 5e012aad85f2ee31d7de5c21d63ccd2702d63db2 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 15 Apr 2014 12:24:55 +0200 Subject: [PATCH 2/7] [SCSI] don't reference freed command in scsi_init_sgtable Patch commit 0479633686d370303e3430256ace4bd5f7f138dc Author: Christoph Hellwig Date: Thu Feb 20 14:20:55 2014 -0800 [SCSI] do not manipulate device reference counts in scsi_get/put_command Introduced a use after free: when scsi_init_io fails we have to release our device reference, but we do this trying to reference the just freed command. Add a local scsi_device pointer to fix this. Fixes: 0479633686d370303e3430256ace4bd5f7f138dc Reported-by: Sander Eikelenboom Signed-off-by: Christoph Hellwig Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 65a123d9c676..54eff6a79fb8 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1044,6 +1044,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb, */ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask) { + struct scsi_device *sdev = cmd->device; struct request *rq = cmd->request; int error = scsi_init_sgtable(rq, &cmd->sdb, gfp_mask); @@ -1091,7 +1092,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask) scsi_release_buffers(cmd); cmd->request->special = NULL; scsi_put_command(cmd); - put_device(&cmd->device->sdev_gendev); + put_device(&sdev->sdev_gendev); return error; } EXPORT_SYMBOL(scsi_init_io); From 68c03d9193f55dad93036f439b94912c5003a173 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 15 Apr 2014 12:24:56 +0200 Subject: [PATCH 3/7] [SCSI] don't reference freed command in scsi_prep_return Patch commit 0479633686d370303e3430256ace4bd5f7f138dc Author: Christoph Hellwig Date: Thu Feb 20 14:20:55 2014 -0800 [SCSI] do not manipulate device reference counts in scsi_get/put_command Introduced a use after free:I in the kill case of scsi_prep_return we have to release our device reference, but we do this trying to reference the just freed command. Use the local sdev pointer instead. Fixes: 0479633686d370303e3430256ace4bd5f7f138dc Reported-by: Joe Lawrence Signed-off-by: Christoph Hellwig Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 54eff6a79fb8..7fa54fe51f63 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1274,7 +1274,7 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret) struct scsi_cmnd *cmd = req->special; scsi_release_buffers(cmd); scsi_put_command(cmd); - put_device(&cmd->device->sdev_gendev); + put_device(&sdev->sdev_gendev); req->special = NULL; } break; From d555a2abf3481f81303d835046a5ec2c4fb3ca8e Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Fri, 28 Mar 2014 10:50:17 -0700 Subject: [PATCH 4/7] [SCSI] Fix spurious request sense in error handling We unconditionally execute scsi_eh_get_sense() to make sure all failed commands that should have sense attached, do. However, the routine forgets that some commands, because of the way they fail, will not have any sense code ... we should not bother them with a REQUEST_SENSE command. Fix this by testing to see if we actually got a CHECK_CONDITION return and skip asking for sense if we don't. Tested-by: Alan Stern Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 771c16bfdbac..d020149ea8d4 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1157,6 +1157,15 @@ int scsi_eh_get_sense(struct list_head *work_q, __func__)); break; } + if (status_byte(scmd->result) != CHECK_CONDITION) + /* + * don't request sense if there's no check condition + * status because the error we're processing isn't one + * that has a sense code (and some devices get + * confused by sense requests out of the blue) + */ + continue; + SCSI_LOG_ERROR_RECOVERY(2, scmd_printk(KERN_INFO, scmd, "%s: requesting sense\n", current->comm)); From 644373a4219add42123df69c8b7ce6a918475ccd Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 28 Mar 2014 10:51:15 -0700 Subject: [PATCH 5/7] [SCSI] Fix command result state propagation We're seeing a case where the contents of scmd->result isn't being reset after a SCSI command encounters an error, is resubmitted, times out and then gets handled. The error handler acts on the stale result of the previous error instead of the timeout. Fix this by properly zeroing the scmd->status before the command is resubmitted. Signed-off-by: Alan Stern Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 1 + drivers/scsi/scsi_lib.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index d020149ea8d4..2953bfa92da7 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -924,6 +924,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses, memset(scmd->cmnd, 0, BLK_MAX_CDB); memset(&scmd->sdb, 0, sizeof(scmd->sdb)); scmd->request->next_rq = NULL; + scmd->result = 0; if (sense_bytes) { scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE, diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 7fa54fe51f63..9db097a28a74 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -137,6 +137,7 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy) * lock such that the kblockd_schedule_work() call happens * before blk_cleanup_queue() finishes. */ + cmd->result = 0; spin_lock_irqsave(q->queue_lock, flags); blk_requeue_request(q, cmd->request); kblockd_schedule_work(q, &device->requeue_work); From 7daf480483e60898f30e0a2a84fecada7a7cfac0 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 31 Mar 2014 16:37:34 +0200 Subject: [PATCH 6/7] [SCSI] Fix USB deadlock caused by SCSI error handling USB requires that every command be aborted first before we escalate to reset. In particular, USB will deadlock if we try to reset first before aborting the command. Unfortunately, the flag we use to tell if a command has already been aborted: SCSI_EH_ABORT_SCHEDULED is not cleared properly leading to cases where we can requeue a command with the flag set and proceed immediately to reset if it fails (thus causing USB to deadlock). Fix by clearing the SCSI_EH_ABORT_SCHEDULED flag if it has been set. Which means this will be the second time scsi_abort_command() has been called for the same command. IE the first abort went out, did its thing, but now the same command has timed out again. So this flag gets cleared, and scsi_abort_command() returns FAILED, and _no_ asynchronous abort is being scheduled. scsi_times_out() will then proceed to call scsi_eh_scmd_add(). But as we've cleared the SCSI_EH_ABORT_SCHEDULED flag the SCSI_EH_CANCEL_CMD flag will continue to be set, and the command will be aborted with the main SCSI EH routine. Reported-by: Alan Stern Tested-by: Andreas Reis Signed-off-by: Hannes Reinecke Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 2953bfa92da7..ad064d2d730b 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -189,6 +189,7 @@ scsi_abort_command(struct scsi_cmnd *scmd) /* * Retry after abort failed, escalate to next level. */ + scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED; SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, "scmd %p previous abort failed\n", scmd)); From c69e6f812bab0d5442b40e2f1bfbca48d40bc50b Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 10 Apr 2014 13:36:11 -0700 Subject: [PATCH 7/7] [SCSI] More USB deadlock fixes This patch fixes a corner case in the previous USB Deadlock fix patch (12023e7 [SCSI] Fix USB deadlock caused by SCSI error handling). The scenario is abort command, set flag, abort completes, send TUR, TUR doesn't return, so we now try to abort the TUR, but scsi_abort_eh_cmnd() will skip the abort because the flag is set and move straight to reset. Reviewed-by: Hannes Reinecke Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index ad064d2d730b..f17aa7aa7879 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -921,6 +921,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses, ses->prot_op = scmd->prot_op; scmd->prot_op = SCSI_PROT_NORMAL; + scmd->eh_eflags = 0; scmd->cmnd = ses->eh_cmnd; memset(scmd->cmnd, 0, BLK_MAX_CDB); memset(&scmd->sdb, 0, sizeof(scmd->sdb));