From: Rob Evers <revers@redhat.com>
Date: Thu, 16 Jul 2009 13:56:42 -0400
Subject: [scsi] mptfusion: fix OOPS in failover path
Message-id: 20090716175204.3800.82285.sendpatchset@localhost.localdomain
O-Subject: [RHEL5.4 PATCH] mptfusion: fix OOPS in SCSI failover for MPT drivers
Bugzilla: 504835
RH-Acked-by: Mike Christie <mchristi@redhat.com>
RH-Acked-by: Pete Zaitcev <zaitcev@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=504835

description:

    Fix race condition that causes double free if a request in mptfustion
    See bugzilla report comment 1 for full details.

upstream status:

    According to the bugzilla, planned upstream restructuring of mptfusion
    upstream outdates this patch upstream.

brew-build id:

    1887975

testing:

    The bugzilla report gives a detailed analysis of the problem, and the
    solution, though only indicates that the problem was found during testing.

    Assuming that the testing that found the problem was also done after
    the fix was found.

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index d201181..60a4a62 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1031,7 +1031,7 @@ mpt_put_msg_frame_hi_pri(u8 cb_idx, MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
- *	mpt_free_msg_frame - Place MPT request frame back on FreeQ.
+ *	__mpt_free_msg_frame - Place MPT request frame back on FreeQ.
  *	@ioc: Pointer to MPT adapter structure
  *	@mf: Pointer to MPT request frame
  *
@@ -1039,18 +1039,15 @@ mpt_put_msg_frame_hi_pri(u8 cb_idx, MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
  *	FreeQ.
  */
 void
-mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
+__mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
 {
-	unsigned long flags;
-
+	BUG_ON(mf->u.frame.linkage.arg1 == 0xdeadbeaf);
 	/*  Put Request back on FreeQ!  */
-	spin_lock_irqsave(&ioc->FreeQlock, flags);
 	mf->u.frame.linkage.arg1 = 0xdeadbeaf; /* signature to know if this mf is freed */
 	list_add_tail(&mf->u.frame.linkage.list, &ioc->FreeQ);
 #ifdef MFCNT
 	ioc->mfcnt--;
 #endif
-	spin_unlock_irqrestore(&ioc->FreeQlock, flags);
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -7877,7 +7874,7 @@ EXPORT_SYMBOL(mpt_device_driver_deregister);
 EXPORT_SYMBOL(mpt_get_msg_frame);
 EXPORT_SYMBOL(mpt_put_msg_frame);
 EXPORT_SYMBOL(mpt_put_msg_frame_hi_pri);
-EXPORT_SYMBOL(mpt_free_msg_frame);
+EXPORT_SYMBOL(__mpt_free_msg_frame);
 EXPORT_SYMBOL(mpt_send_handshake_request);
 EXPORT_SYMBOL(mpt_verify_adapter);
 EXPORT_SYMBOL(mpt_GetIocState);
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 0c77460..b76d900 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -919,7 +919,7 @@ extern void	 mpt_reset_deregister(u8 cb_idx);
 extern int	 mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, u8 cb_idx);
 extern void	 mpt_device_driver_deregister(u8 cb_idx);
 extern MPT_FRAME_HDR	*mpt_get_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc);
-extern void	 mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
+extern void	 __mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
 extern void	 mpt_put_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
 extern void	 mpt_put_msg_frame_hi_pri(u8 cb_idx, MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
 
@@ -936,6 +936,14 @@ extern int	 mptbase_sas_persist_operation(MPT_ADAPTER *ioc, u8 persist_opcode);
 extern int	 mpt_raid_phys_disk_pg0(MPT_ADAPTER *ioc, u8 phys_disk_num, pRaidPhysDiskPage0_t phys_disk);
 extern void     mpt_halt_firmware(MPT_ADAPTER *ioc);
 
+static inline void
+mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&ioc->FreeQlock, flags);
+	__mpt_free_msg_frame(ioc, mf);
+	spin_unlock_irqrestore(&ioc->FreeQlock, flags);
+}
 
 /*
  *  Public data decl's...
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index ac3973b..6479dc6 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1655,6 +1655,18 @@ mptscsih_IssueTaskMgmt(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 id, int lun, i
 	}
 
 	if(mptscsih_tm_wait_for_completion(hd, timeout) == FAILED) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&ioc->FreeQlock, flags);
+		if(hd->tmPending == 0) {
+			spin_unlock_irqrestore(&ioc->FreeQlock, flags);
+			goto out_success;
+		}
+		__mpt_free_msg_frame(ioc, mf);
+		hd->tmPending = 0;
+		hd->tmState = TM_STATE_NONE;
+		spin_unlock_irqrestore(&ioc->FreeQlock, flags);
+
 		dfailprintk(ioc, printk(MYIOC_s_ERR_FMT "task management request TIMED OUT!"
 			" (hd %p, ioc %p, mf %p) \n", ioc->name, hd,
 			ioc, mf));
@@ -1663,9 +1675,11 @@ mptscsih_IssueTaskMgmt(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 id, int lun, i
 		retval = mpt_HardResetHandler(ioc, CAN_SLEEP);
 		dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT "rc=%d \n",
 			 ioc->name, retval));
-		goto fail_out;
+		return FAILED;
 	}
 
+ out_success:
+
 	/*
 	 * Handle success case, see if theres a non-zero ioc_status.
 	 */
@@ -2098,6 +2112,7 @@ mptscsih_taskmgmt_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *m
 	u16			 iocstatus;
 	u8			 tmType;
 	u32			 termination_count;
+	int retval = 1;
 
 	dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT "TaskMgmt completed (mf=%p,mr=%p)\n",
 	    ioc->name, mf, mr));
@@ -2174,12 +2189,15 @@ mptscsih_taskmgmt_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *m
 
  out:
 	spin_lock_irqsave(&ioc->FreeQlock, flags);
-	hd->tmPending = 0;
-	hd->tmState = TM_STATE_NONE;
+	if (hd->tmPending) {
+		hd->tmPending = 0;
+		hd->tmState = TM_STATE_NONE;
+	} else
+		retval = 0;	
 	hd->tm_iocstatus = iocstatus;
 	spin_unlock_irqrestore(&ioc->FreeQlock, flags);
 
-	return 1;
+	return retval;
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/