From: shefty <shefty@ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86>
Date: Wed, 26 Aug 2009 16:45:53 +0000 (+0000)
Subject: librdmacm: fix race in ucma_init()
X-Git-Url: http://git.etherboot.org/mirror/winof/.git/commitdiff_plain/7b16cdcc31a2a7778011bdbb0469e29123814829

librdmacm: fix race in ucma_init()

There's a potential race with ucma_init() and calls that check whether
the library is ready for use.  For example, in rdma_create_id, this
check is performed:

	hr = cma_dev_cnt ? 0 : ucma_init();
	if (hr) {
		return hr;
	}

Since the check for cma_dev_cnt is outside of any synchronization, if it
is non-zeroy, then the code will assume that initialization is complete.
However, ucma_init() can set cma_dev_cnt to non-zeroy before it has finished
executing.  The full impact of this race is unknown, but it can't be good.
In the worst case, the application may crash.

Fix this by ensuring that all initialization is complete before cma_dev_cnt
is incremented.  Note that ucma_init() provides synchronization to protect
against duplicate threads handling init.  The cma_dev_cnt check is used to
avoid synchronization on multiple calls into the library after initialization.

Signed-off-by: Sean Hefty <sean.hefty@intel.com>


git-svn-id: svn://openib.tc.cornell.edu/gen1/trunk@2383 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86
---

diff --git a/ulp/librdmacm/src/cma.cpp b/ulp/librdmacm/src/cma.cpp
index e7322342..7d31c30d 100644
--- a/ulp/librdmacm/src/cma.cpp
+++ b/ulp/librdmacm/src/cma.cpp
@@ -91,63 +91,49 @@ struct cma_event {
 static struct cma_device *cma_dev_array;
 static int cma_dev_cnt;
 
-static void ucma_cleanup(void)
-{
-	if (cma_dev_cnt > 0) {
-		while (cma_dev_cnt > 0) {
-			ibv_close_device(cma_dev_array[--cma_dev_cnt].verbs);
-		}
-		delete cma_dev_array;
-		cma_dev_cnt = 0;
-	}
-	if (windata.prov != NULL) {
-		ibvw_release_windata(&windata, IBVW_WINDATA_VERSION);
-		windata.prov = NULL;
-	}
-}
-
 static int ucma_init(void)
 {
 	struct ibv_device **dev_list = NULL;
 	struct cma_device *cma_dev;
 	struct ibv_device_attr attr;
-	int i, ret;
+	int i, ret, dev_cnt;
 
 	EnterCriticalSection(&lock);
-	if (cma_dev_cnt > 0) {
+	if (cma_dev_cnt) {
 		goto out;
 	}
 
 	ret = ibvw_get_windata(&windata, IBVW_WINDATA_VERSION);
 	if (ret) {
-		goto err;
+		goto err1;
 	}
 
-	dev_list = ibv_get_device_list(&cma_dev_cnt);
+	dev_list = ibv_get_device_list(&dev_cnt);
 	if (dev_list == NULL) {
 		ret = -1;
-		goto err;
+		goto err2;
 	}
 
-	cma_dev_array = new struct cma_device[cma_dev_cnt];
+	cma_dev_array = new struct cma_device[dev_cnt];
 	if (cma_dev_array == NULL) {
 		ret = -1;
-		goto err;
+		goto err3;
 	}
 
-	for (i = 0; dev_list[i]; ++i) {
+	for (i = 0; dev_list[i];) {
 		cma_dev = &cma_dev_array[i];
 
 		cma_dev->guid = ibv_get_device_guid(dev_list[i]);
 		cma_dev->verbs = ibv_open_device(dev_list[i]);
 		if (cma_dev->verbs == NULL) {
 			ret = -1;
-			goto err;
+			goto err4;
 		}
 
+		++i;
 		ret = ibv_query_device(cma_dev->verbs, &attr);
 		if (ret) {
-			goto err;
+			goto err4;
 		}
 
 		cma_dev->port_cnt = attr.phys_port_cnt;
@@ -155,16 +141,22 @@ static int ucma_init(void)
 		cma_dev->max_responder_resources = (uint8_t) attr.max_qp_rd_atom;
 	}
 	ibv_free_device_list(dev_list);
+	cma_dev_cnt = dev_cnt;
 out:
 	LeaveCriticalSection(&lock);
 	return 0;
 
-err:
-	ucma_cleanup();
-	LeaveCriticalSection(&lock);
-	if (dev_list) {
-		ibv_free_device_list(dev_list);
+err4:
+	while (i) {
+		ibv_close_device(cma_dev_array[--i].verbs);
 	}
+	delete cma_dev_array;
+err3:
+	ibv_free_device_list(dev_list);
+err2:
+	ibvw_release_windata(&windata, IBVW_WINDATA_VERSION);
+err1:
+	LeaveCriticalSection(&lock);
 	return ret;
 }