[ofw] [PATCH] winverbs: fix cleanup in error handling cases

[Sean Hefty]

Running the ndmw test results in a crash in the winverbs driver.  The
crash is caused by improper cleanup in the winverbs driver as a result
of a failure trying to allocate a memory window.

In studying the crash, the same situation could arise if other objects
also fail to be created (QPs, CQs, PDs, etc.)  To fix the crash and
simplify the code, explicit calls to used to increment the reference
count on referenced objects, rather than simply holding onto the
reference that was acquired when the object was validated.  (The latter
is slightly more efficient, but results in less maintainable code
once any fix is applied.)

Signed-off-by: Sean Hefty <sean.hefty at intel.com>
---
I will continue testing with this fix, and if no other issues come up,
will commit to the trunk tonight or early tomorrow.  The bug only affects
error paths, but the fix changes all call paths.

Also, after I verified that this fixed the crash, userspace was changed
to work around the missing MW support.

diff -up -r -X \mshefty\scm\winof\trunk\docs\dontdiff.txt -I '\$Id:' trunk\core\winverbs/kernel/wv_pd.c
branches\winverbs\core\winverbs/kernel/wv_pd.c
--- trunk\core\winverbs/kernel/wv_pd.c	2009-06-11 10:32:48.299250000 -0700
+++ branches\winverbs\core\winverbs/kernel/wv_pd.c	2009-07-30 11:24:25.821503000 -0700
@@ -94,6 +94,7 @@ static NTSTATUS WvPdAlloc(WV_DEVICE *pDe
 		goto err;
 	}
 
+	WvDeviceGet(pDevice);
 	pd->pDevice = pDevice;
 	pd->pVerbs = pDevice->pVerbs;
 	*ppPd = pd;
@@ -144,7 +145,7 @@ void WvPdAllocate(WV_PROVIDER *pProvider
 	InsertHeadList(&dev->PdList, &pd->Entry);
 	KeReleaseGuardedMutex(&pProvider->Lock);
 
-	WvProviderEnableRemove(pProvider);
+	WvDeviceRelease(dev);
 	outid->VerbInfo = verbsData.status;
 	WdfRequestCompleteWithInformation(Request, status, outlen);
 	return;
@@ -426,6 +427,9 @@ void WvMwAllocate(WV_PROVIDER *pProvider
 		goto err2;
 	}
 
+	WvPdGet(pd);
+	mw->pPd = pd;
+
 	WvInitVerbsData(&verbsData, inid->VerbInfo, inlen - sizeof(WV_IO_ID),
 					outlen - sizeof(WV_IO_ID), inid + 1);
 	ib_status = pd->pVerbs->create_mw(pd->hVerbsPd, &outid->Data, &mw->hVerbsMw,
@@ -439,20 +443,19 @@ void WvMwAllocate(WV_PROVIDER *pProvider
 	outid->Id = IndexListInsertHead(&pProvider->MwIndex, mw);
 	if (outid->Id == 0) {
 		status = STATUS_NO_MEMORY;
-		goto err3;
+		goto err4;
 	}
 	InsertHeadList(&pd->MwList, &mw->Entry);
 	KeReleaseGuardedMutex(&pProvider->Lock);
 
-	mw->pPd = pd;
-
-	WvProviderEnableRemove(pProvider);
+	WvPdRelease(pd);
 	outid->VerbInfo = verbsData.status;
 	WdfRequestCompleteWithInformation(Request, status, outlen);
 	return;
 
-err3:
+err4:
 	KeReleaseGuardedMutex(&pProvider->Lock);
+err3:
 	WvMwFree(mw);
 err2:
 	WvPdRelease(pd);
@@ -553,6 +556,9 @@ void WvAhCreate(WV_PROVIDER *pProvider, 
 		goto err2;
 	}
 
+	WvPdGet(pd);
+	ah->pPd = pd;
+
 	WvVerbsConvertAv(&av, &pinAv->AddressVector);
 	WvInitVerbsData(&verbsData, pinAv->Id.VerbInfo, inlen - sizeof(WV_IO_AH_CREATE),
 					outlen - sizeof(WV_IO_AH_CREATE), pinAv + 1);
@@ -566,20 +572,19 @@ void WvAhCreate(WV_PROVIDER *pProvider, 
 	poutAv->Id.Id = IndexListInsertHead(&pProvider->AhIndex, ah);
 	if (poutAv->Id.Id == 0) {
 		status = STATUS_NO_MEMORY;
-		goto err3;
+		goto err4;
 	}
 	InsertHeadList(&pd->AhList, &ah->Entry);
 	KeReleaseGuardedMutex(&pProvider->Lock);
 
-	ah->pPd = pd;
-
-	WvProviderEnableRemove(pProvider);
+	WvPdRelease(pd);
 	poutAv->Id.VerbInfo = verbsData.status;
 	WdfRequestCompleteWithInformation(Request, status, outlen);
 	return;
 
-err3:
+err4:
 	KeReleaseGuardedMutex(&pProvider->Lock);
+err3:
 	WvAhFree(ah);
 err2:
 	WvPdRelease(pd);
diff -up -r -X \mshefty\scm\winof\trunk\docs\dontdiff.txt -I '\$Id:' trunk\core\winverbs/kernel/wv_qp.c
branches\winverbs\core\winverbs/kernel/wv_qp.c
--- trunk\core\winverbs/kernel/wv_qp.c	2009-06-11 10:32:48.346125000 -0700
+++ branches\winverbs\core\winverbs/kernel/wv_qp.c	2009-07-29 17:07:20.961012900 -0700
@@ -528,7 +528,6 @@ void WvQpFree(WV_QUEUE_PAIR *pQp)
 		if (mc->hVerbsMc != NULL) {
 			pQp->pVerbs->detach_mcast(mc->hVerbsMc);
 		}
-		WvQpPut(pQp);
 		ExFreePool(mc);
 	}
 
@@ -663,7 +662,7 @@ void WvQpAttach(WV_PROVIDER *pProvider, 
 	InsertHeadList(&qp->McList, &pmc->Entry);
 	KeReleaseGuardedMutex(&qp->pPd->Lock);
 
-	WvProviderEnableRemove(pProvider);
+	WvQpRelease(qp);
 	WdfRequestComplete(Request, STATUS_SUCCESS);
 	return;
 
@@ -707,18 +706,17 @@ void WvQpDetach(WV_PROVIDER *pProvider, 
 			if (pmc->hVerbsMc != NULL) {
 				qp->pVerbs->detach_mcast(pmc->hVerbsMc);
 			}
-			WvQpPut(qp);
-			WvQpRelease(qp);
 
 			ExFreePool(pmc);
 			status = STATUS_SUCCESS;
-			goto complete;
+			goto release;
 		}
 	}
 	KeReleaseGuardedMutex(&qp->pPd->Lock);
-	WvQpRelease(qp);
 	status = STATUS_NOT_FOUND;
 
+release:
+	WvQpRelease(qp);
 complete:
 	WdfRequestComplete(Request, status);
 }
diff -up -r -X \mshefty\scm\winof\trunk\docs\dontdiff.txt -I '\$Id:' trunk\core\winverbs/kernel/wv_srq.c
branches\winverbs\core\winverbs/kernel/wv_srq.c
--- trunk\core\winverbs/kernel/wv_srq.c	2009-06-11 10:32:48.330500000 -0700
+++ branches\winverbs\core\winverbs/kernel/wv_srq.c	2009-07-29 17:00:36.601587200 -0700
@@ -123,6 +123,7 @@ static NTSTATUS WvSrqAlloc(WV_PROTECTION
 		goto err2;
 	}
 
+	WvPdGet(pPd);
 	srq->pPd = pPd;
 	srq->pProvider = pPd->pDevice->pProvider;
 	srq->pVerbs = pPd->pVerbs;
@@ -178,7 +179,7 @@ void WvSrqCreate(WV_PROVIDER *pProvider,
 	InsertHeadList(&pd->SrqList, &srq->Entry);
 	KeReleaseGuardedMutex(&pProvider->Lock);
 
-	WvProviderEnableRemove(pProvider);
+	WvPdRelease(pd);
 	outAttr->Id.VerbInfo = verbsData.status;
 	WdfRequestCompleteWithInformation(Request, status, outlen);
 	return;