[PATCH 4.14,4.15 1/2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

[PATCH 4.14,4.15 1/2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

[Mauricio Faria de Oliveira]

commit 9ff549ffb4fb4cc9a4b24d1de9dc3e68287797c4 upstream.

This patch adds checks for 'ioc->remove_host' in the SCSI error handlers, so
not to access pointers/resources potentially freed in the PCI shutdown/module
unload path.  The error handlers may be invoked after shutdown/unload,
depending on other components.

This problem was observed with kexec on a system with a mpt3sas based adapter
and an infiniband adapter which takes long enough to shutdown:

The mpt3sas driver finished shutting down / disabled interrupt handling, thus
some commands have not finished and timed out.

Since the system was still running (waiting for the infiniband adapter to
shutdown), the scsi error handler for task abort of mpt3sas was invoked, and
hit an oops -- either in scsih_abort() because 'ioc->scsi_lookup' was NULL
without commit dbec4c9040ed ("scsi: mpt3sas: lockless command submission"), or
later up in scsih_host_reset() (with or without that commit), because it
eventually called mpt3sas_base_get_iocstate().

After the above commit, the oops in scsih_abort() does not occur anymore
(_scsih_scsi_lookup_find_by_scmd() is no longer called), but that commit is
too big and out of the scope of linux-stable, where this patch might help, so
still go for the changes.

Also, this might help to prevent similar errors in the future, in case code
changes and possibly tries to access freed stuff.

Note the fix in scsih_host_reset() is still important anyway.

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Acked-by: Sreekanth Reddy <Sreekanth.Reddy@broadcom.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index b258f21..4adc7c7 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2998,7 +2998,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	_scsih_tm_display_info(ioc, scmd);
 
 	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+	    ioc->remove_host) {
 		sdev_printk(KERN_INFO, scmd->device,
 			"device been deleted! scmd(%p)\n", scmd);
 		scmd->result = DID_NO_CONNECT << 16;
@@ -3060,7 +3061,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	_scsih_tm_display_info(ioc, scmd);
 
 	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+	    ioc->remove_host) {
 		sdev_printk(KERN_INFO, scmd->device,
 			"device been deleted! scmd(%p)\n", scmd);
 		scmd->result = DID_NO_CONNECT << 16;
@@ -3122,7 +3124,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	_scsih_tm_display_info(ioc, scmd);
 
 	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+	    ioc->remove_host) {
 		starget_printk(KERN_INFO, starget, "target been deleted! scmd(%p)\n",
 			scmd);
 		scmd->result = DID_NO_CONNECT << 16;
@@ -3179,7 +3182,7 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	    ioc->name, scmd);
 	scsi_print_command(scmd);
 
-	if (ioc->is_driver_loading) {
+	if (ioc->is_driver_loading || ioc->remove_host) {
 		pr_info(MPT3SAS_FMT "Blocking the host reset\n",
 		    ioc->name);
 		r = FAILED;
-- 
1.8.3.1

[PATCH 4.14,4.15 2/2] scsi: mpt3sas: wait for and flush running commands on shutdown/unload

[Mauricio Faria de Oliveira]

From: Sreekanth Reddy <sreekanth.reddy@broadcom.com>

commit c666d3be99c000bb889a33353e9be0fa5808d3de upstream.

This patch finishes all outstanding SCSI IO commands (but not other commands,
e.g., task management) in the shutdown and unload paths.

It first waits for the commands to complete (this is done after setting
'ioc->remove_host = 1 ', which prevents new commands to be queued) then it
flushes commands that might still be running.

This avoids triggering error handling (e.g., abort command) for all commands
possibly completed by the adapter after interrupts disabled.

[mauricfo: introduced something in commit message.]

Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Tested-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
[mauricfo: backport to linux-4.15.y (a few updates to context lines)]
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  |  8 ++++----
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  3 +++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 10 +++++++++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 8027de4..f43b514 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6289,14 +6289,14 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 }
 
 /**
- * _wait_for_commands_to_complete - reset controller
+ * mpt3sas_wait_for_commands_to_complete - reset controller
  * @ioc: Pointer to MPT_ADAPTER structure
  *
  * This function waiting(3s) for all pending commands to complete
  * prior to putting controller in reset.
  */
-static void
-_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
+void
+mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
 {
 	u32 ioc_state;
 	unsigned long flags;
@@ -6375,7 +6375,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 			is_fault = 1;
 	}
 	_base_reset_handler(ioc, MPT3_IOC_PRE_RESET);
-	_wait_for_commands_to_complete(ioc);
+	mpt3sas_wait_for_commands_to_complete(ioc);
 	_base_mask_interrupts(ioc);
 	r = _base_make_ioc_ready(ioc, type);
 	if (r)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 60f42ca..69022b1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1435,6 +1435,9 @@ void mpt3sas_base_update_missing_delay(struct MPT3SAS_ADAPTER *ioc,
 
 int mpt3sas_port_enable(struct MPT3SAS_ADAPTER *ioc);
 
+void
+mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc);
+
 
 /* scsih shared API */
 u8 mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 4adc7c7..741b0a2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4614,7 +4614,7 @@ static int _scsih_set_satl_pending(struct scsi_cmnd *scmd, bool pending)
 		_scsih_set_satl_pending(scmd, false);
 		mpt3sas_base_free_smid(ioc, smid);
 		scsi_dma_unmap(scmd);
-		if (ioc->pci_error_recovery)
+		if (ioc->pci_error_recovery || ioc->remove_host)
 			scmd->result = DID_NO_CONNECT << 16;
 		else
 			scmd->result = DID_RESET << 16;
@@ -9904,6 +9904,10 @@ static void scsih_remove(struct pci_dev *pdev)
 	unsigned long flags;
 
 	ioc->remove_host = 1;
+
+	mpt3sas_wait_for_commands_to_complete(ioc);
+	_scsih_flush_running_cmds(ioc);
+
 	_scsih_fw_event_cleanup_queue(ioc);
 
 	spin_lock_irqsave(&ioc->fw_event_lock, flags);
@@ -9980,6 +9984,10 @@ static void scsih_remove(struct pci_dev *pdev)
 	unsigned long flags;
 
 	ioc->remove_host = 1;
+
+	mpt3sas_wait_for_commands_to_complete(ioc);
+	_scsih_flush_running_cmds(ioc);
+
 	_scsih_fw_event_cleanup_queue(ioc);
 
 	spin_lock_irqsave(&ioc->fw_event_lock, flags);
-- 
1.8.3.1

Re: [PATCH 4.14,4.15 2/2] scsi: mpt3sas: wait for and flush running commands on shutdown/unload

[Greg KH]

On Thu, Mar 08, 2018 at 11:55:13AM -0300, Mauricio Faria de Oliveira wrote:
> From: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
> 
> commit c666d3be99c000bb889a33353e9be0fa5808d3de upstream.
> 
> This patch finishes all outstanding SCSI IO commands (but not other commands,
> e.g., task management) in the shutdown and unload paths.
> 
> It first waits for the commands to complete (this is done after setting
> 'ioc->remove_host = 1 ', which prevents new commands to be queued) then it
> flushes commands that might still be running.
> 
> This avoids triggering error handling (e.g., abort command) for all commands
> possibly completed by the adapter after interrupts disabled.
> 
> [mauricfo: introduced something in commit message.]
> 
> Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
> Tested-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> [mauricfo: backport to linux-4.15.y (a few updates to context lines)]

Also does not apply to 4.14.y :(

Re: [PATCH 4.14,4.15 1/2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

[Greg KH]

On Thu, Mar 08, 2018 at 11:55:12AM -0300, Mauricio Faria de Oliveira wrote:
> commit 9ff549ffb4fb4cc9a4b24d1de9dc3e68287797c4 upstream.
> 
> This patch adds checks for 'ioc->remove_host' in the SCSI error handlers, so
> not to access pointers/resources potentially freed in the PCI shutdown/module
> unload path.  The error handlers may be invoked after shutdown/unload,
> depending on other components.
> 
> This problem was observed with kexec on a system with a mpt3sas based adapter
> and an infiniband adapter which takes long enough to shutdown:
> 
> The mpt3sas driver finished shutting down / disabled interrupt handling, thus
> some commands have not finished and timed out.
> 
> Since the system was still running (waiting for the infiniband adapter to
> shutdown), the scsi error handler for task abort of mpt3sas was invoked, and
> hit an oops -- either in scsih_abort() because 'ioc->scsi_lookup' was NULL
> without commit dbec4c9040ed ("scsi: mpt3sas: lockless command submission"), or
> later up in scsih_host_reset() (with or without that commit), because it
> eventually called mpt3sas_base_get_iocstate().
> 
> After the above commit, the oops in scsih_abort() does not occur anymore
> (_scsih_scsi_lookup_find_by_scmd() is no longer called), but that commit is
> too big and out of the scope of linux-stable, where this patch might help, so
> still go for the changes.
> 
> Also, this might help to prevent similar errors in the future, in case code
> changes and possibly tries to access freed stuff.
> 
> Note the fix in scsih_host_reset() is still important anyway.
> 
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> Acked-by: Sreekanth Reddy <Sreekanth.Reddy@broadcom.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Does not apply to the 4.14.y tree :(

Please provide a working backport if you want me to apply it there.

thanks,

greg k-h

Re: [PATCH 4.14,4.15 1/2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

[Mauricio Faria de Oliveira]

Greg,

On 03/09/2018 09:07 PM, Greg KH wrote:
> Does not apply to the 4.14.y tree:(

Sorry. The problem for 4.14.y was the _line number_ of the first hunk.
It turns out it was an important difference between 4.15.y and 4.14.y.

The code contents were exactly the same, but that had to be different.
I'll check better next time.

> Please provide a working backport if you want me to apply it there.

Sure, I just sent a 'PATCH 4.14 v2'.

Thanks,
Mauricio