RE: [PATCH v2 1/2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----Original Message-----
From: Mauricio Faria de Oliveira [mailto:mauricfo@xxxxxxxxxxxxxxxxxx]
Sent: Saturday, February 17, 2018 4:10 AM
To: linux-scsi@xxxxxxxxxxxxxxx; Bart.VanAssche@xxxxxxx;
sreekanth.reddy@xxxxxxxxxxxx
Cc: sathya.prakash@xxxxxxxxxxxx; chaitra.basappa@xxxxxxxxxxxx;
suganath-prabu.subramani@xxxxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx;
martin.petersen@xxxxxxxxxx; dougmill@xxxxxxxxxxxxxxxxxx
Subject: [PATCH v2 1/2] scsi: mpt3sas: fix oops in error handlers after
shutdown/unload

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 dbec4c90
"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 commit dbec4c90, 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@xxxxxxxxxxxxxxxxxx>
---
 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 74fca18..5ab3caf 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2835,7 +2835,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;
@@ -2898,7 +2899,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;
@@ -2961,7 +2963,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;
@@ -3019,7 +3022,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


Acked-by: Sreekanth Reddy <Sreekanth.Reddy@xxxxxxxxxxxx>

Thanks,
Sreekanth



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux