6909450 Race condition in iscsit session code allows duplicate sessions
authorPriya Krishnan <Priya.Krishnan@Sun.COM>
Sun, 27 Jun 2010 07:17:36 -0400
changeset 12702 2169bee4a248
parent 12701 8c9362e17ee4
child 12703 fa978d26fcd6
6909450 Race condition in iscsit session code allows duplicate sessions
usr/src/uts/common/io/comstar/port/iscsit/iscsit.c
usr/src/uts/common/io/comstar/port/iscsit/iscsit_login.c
--- a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c	Sat Jun 26 15:12:38 2010 -0700
+++ b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c	Sun Jun 27 07:17:36 2010 -0400
@@ -116,6 +116,8 @@
 
 boolean_t	iscsit_sm_logging = B_FALSE;
 
+kmutex_t	login_sm_session_mutex;
+
 static idm_status_t iscsit_init(dev_info_t *dip);
 static idm_status_t iscsit_enable_svc(iscsit_hostinfo_t *hostinfo);
 static void iscsit_disable_svc(void);
@@ -271,6 +273,7 @@
 
 	mutex_init(&iscsit_rxpdu_queue_monitor_mutex, NULL,
 	    MUTEX_DRIVER, NULL);
+	mutex_init(&login_sm_session_mutex, NULL, MUTEX_DRIVER, NULL);
 	iscsit_rxpdu_queue_monitor_thr_id = NULL;
 	iscsit_rxpdu_queue_monitor_thr_running = B_FALSE;
 	cv_init(&iscsit_rxpdu_queue_monitor_cv, NULL, CV_DEFAULT, NULL);
@@ -299,6 +302,7 @@
 
 	if (rc == 0) {
 		mutex_destroy(&iscsit_rxpdu_queue_monitor_mutex);
+		mutex_destroy(&login_sm_session_mutex);
 		cv_destroy(&iscsit_rxpdu_queue_monitor_cv);
 		mutex_destroy(&iscsit_global.global_state_mutex);
 		rw_destroy(&iscsit_global.global_rwlock);
--- a/usr/src/uts/common/io/comstar/port/iscsit/iscsit_login.c	Sat Jun 26 15:12:38 2010 -0700
+++ b/usr/src/uts/common/io/comstar/port/iscsit/iscsit_login.c	Sun Jun 27 07:17:36 2010 -0400
@@ -193,6 +193,12 @@
 
 uint64_t max_dataseglen_target = ISCSIT_MAX_RECV_DATA_SEGMENT_LENGTH;
 
+/*
+ * global mutex defined in iscsit.c to enforce
+ * login_sm_session_bind as a critical section
+ */
+extern kmutex_t login_sm_session_mutex;
+
 idm_status_t
 iscsit_login_sm_init(iscsit_conn_t *ict)
 {
@@ -1266,6 +1272,18 @@
 	uint8_t			error_detail;
 
 	/*
+	 * The multi-threaded execution of binding login sessions to target
+	 * introduced race conditions in the session creation/binding and
+	 * allowed duplicate sessions to tbe created. The addition of the
+	 * global mutex login_sm_session_mutex makes this function single
+	 * threaded to avoid such race conditions. Although this causes
+	 * a small portion of the login to be serialized, it is unlikely
+	 * that there would be numerous simultaneous logins to become a
+	 * performance issue.
+	 */
+	mutex_enter(&login_sm_session_mutex);
+
+	/*
 	 * Look up target and then check if there are sessions or connections
 	 * that match this request (see below).  Any holds taken on objects
 	 * must be released at the end of the function (let's keep things
@@ -1503,6 +1521,7 @@
 	if (existing_ict != NULL)
 		iscsit_conn_rele(existing_ict);
 
+	mutex_exit(&login_sm_session_mutex);
 	return (IDM_STATUS_SUCCESS);
 
 session_bind_error:
@@ -1517,6 +1536,7 @@
 	 * If session bind fails we will fail the login but don't destroy
 	 * the session until later.
 	 */
+	mutex_exit(&login_sm_session_mutex);
 	return (IDM_STATUS_FAIL);
 }