Browse Source

qcacld-3.0: Acquire SME lock before csr_roam_offload_scan from SME

In a scenario where the below two HDD commands are executed at the
same time from different threads
1. Disconnect which does an RSO Stop and free the pCurRoamProfile
2. Set Blacklist BSSID which does and RSO Update and accessed
the pCurRoamProfile
pCurRoamProfile is accessed in the function csr_roam_offload_scan
after is freed from the other context.
The Disconnect command from HDD is protected under the global SME lock,
however, the set blacklist BSSID path is not protected under SME lock.
There are multiple instances where csr_roam_offload_scan is called
without the SME lock which could lead to similar issues.

Acquire SME lock before csr_roam_offload_scan from callers in
SME/HDD which can be from other threads.

Change-Id: I9666bab0001b56ec01dcf1df0becb36344fb6f9a
CRs-Fixed: 2226423
Vignesh Viswanathan 7 years ago
parent
commit
4e65e8eab6
2 changed files with 128 additions and 78 deletions
  1. 115 71
      core/sme/src/common/sme_api.c
  2. 13 7
      core/sme/src/csr/csr_neighbor_roam.c

+ 115 - 71
core/sme/src/common/sme_api.c

@@ -859,6 +859,7 @@ void sme_update_fine_time_measurement_capab(tHalHandle hal, uint8_t session_id,
 						uint32_t val)
 {
 	tpAniSirGlobal mac_ctx = PMAC_STRUCT(hal);
+	QDF_STATUS status;
 
 	ucfg_wifi_pos_set_ftm_cap(mac_ctx->psoc, val);
 
@@ -873,9 +874,15 @@ void sme_update_fine_time_measurement_capab(tHalHandle hal, uint8_t session_id,
 	}
 
 	/* Inform this RRM IE change to FW */
-	csr_roam_offload_scan(mac_ctx, session_id,
+	status = sme_acquire_global_lock(&mac_ctx->sme);
+	if (QDF_IS_STATUS_SUCCESS(status)) {
+		csr_roam_offload_scan(mac_ctx, session_id,
 			ROAM_SCAN_OFFLOAD_UPDATE_CFG,
 			REASON_CONNECT_IES_CHANGED);
+		sme_release_global_lock(&mac_ctx->sme);
+	} else {
+		sme_err("Failed to acquire SME lock");
+	}
 }
 
 /*
@@ -990,6 +997,7 @@ QDF_STATUS sme_update_roam_params(tHalHandle hal,
 {
 	tpAniSirGlobal mac_ctx = PMAC_STRUCT(hal);
 	struct roam_ext_params *roam_params_dst;
+	QDF_STATUS status;
 	uint8_t i;
 
 	roam_params_dst = &mac_ctx->roam.configParam.roam_params;
@@ -1054,8 +1062,16 @@ QDF_STATUS sme_update_roam_params(tHalHandle hal,
 	default:
 		break;
 	}
-	csr_roam_offload_scan(mac_ctx, session_id, ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-			update_param);
+
+	status = sme_acquire_global_lock(&mac_ctx->sme);
+	if (QDF_IS_STATUS_SUCCESS(status)) {
+		csr_roam_offload_scan(mac_ctx, session_id,
+				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+				      update_param);
+		sme_release_global_lock(&mac_ctx->sme);
+	} else {
+		sme_err("Failed to acquire SME lock");
+	}
 
 	sme_update_scan_roam_params(mac_ctx);
 
@@ -1422,6 +1438,7 @@ QDF_STATUS sme_update_is_ese_feature_enabled(tHalHandle hHal,
 			uint8_t sessionId, const bool isEseIniFeatureEnabled)
 {
 	tpAniSirGlobal pMac = PMAC_STRUCT(hHal);
+	QDF_STATUS status;
 
 	if (pMac->roam.configParam.isEseIniFeatureEnabled ==
 	    isEseIniFeatureEnabled) {
@@ -1444,11 +1461,18 @@ QDF_STATUS sme_update_is_ese_feature_enabled(tHalHandle hHal,
 	if (true == isEseIniFeatureEnabled)
 		sme_update_fast_transition_enabled(hHal, true);
 
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled)
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_ESE_INI_CFG_CHANGED);
-
+	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+		status = sme_acquire_global_lock(&pMac->sme);
+		if (QDF_IS_STATUS_SUCCESS(status)) {
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_ESE_INI_CFG_CHANGED);
+			sme_release_global_lock(&pMac->sme);
+		} else {
+			sme_err("Failed to acquire SME lock");
+			return status;
+		}
+	}
 	return QDF_STATUS_SUCCESS;
 }
 
@@ -1735,10 +1759,7 @@ QDF_STATUS sme_set_ese_roam_scan_channel_list(tHalHandle hHal,
 
 	status = sme_acquire_global_lock(&pMac->sme);
 	if (!QDF_IS_STATUS_SUCCESS(status)) {
-		if (pMac->roam.configParam.isRoamOffloadScanEnabled)
-			csr_roam_offload_scan(pMac, sessionId,
-					ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-					REASON_CHANNEL_LIST_CHANGED);
+		sme_err("Failed to acquire SME lock");
 		return status;
 	}
 	if (NULL != curchnl_list_info->ChannelList) {
@@ -1765,11 +1786,13 @@ QDF_STATUS sme_set_ese_roam_scan_channel_list(tHalHandle hHal,
 			newChannelList, oldChannelList,
 			pNeighborRoamInfo->neighborRoamState);
 	}
-	sme_release_global_lock(&pMac->sme);
+
 	if (pMac->roam.configParam.isRoamOffloadScanEnabled)
 		csr_roam_offload_scan(pMac, sessionId,
 				ROAM_SCAN_OFFLOAD_UPDATE_CFG,
 				REASON_CHANNEL_LIST_CHANGED);
+
+	sme_release_global_lock(&pMac->sme);
 	return status;
 }
 
@@ -6363,13 +6386,14 @@ QDF_STATUS sme_update_roam_scan_n_probes(tHalHandle hHal, uint8_t sessionId,
 			  "%s: gRoamScanNProbes is changed from %d to %d",
 			  __func__, pMac->roam.configParam.nProbes, nProbes);
 		pMac->roam.configParam.nProbes = nProbes;
+
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_NPROBES_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_NPROBES_CHANGED);
-	}
 	return status;
 }
 
@@ -6406,14 +6430,15 @@ QDF_STATUS sme_update_roam_scan_home_away_time(tHalHandle hHal,
 			  nRoamScanHomeAwayTime);
 		pMac->roam.configParam.nRoamScanHomeAwayTime =
 			nRoamScanHomeAwayTime;
+
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled &&
+						bSendOffloadCmd) {
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_HOME_AWAY_TIME_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled &&
-					bSendOffloadCmd) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_HOME_AWAY_TIME_CHANGED);
-	}
 	return status;
 }
 
@@ -6537,13 +6562,14 @@ QDF_STATUS sme_update_roam_rssi_diff(tHalHandle hHal, uint8_t sessionId,
 							     [sessionId].
 							    neighborRoamState));
 		pMac->roam.configParam.RoamRssiDiff = RoamRssiDiff;
+
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled)
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_RSSI_DIFF_CHANGED);
+
 		sme_release_global_lock(&pMac->sme);
 	}
-
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled)
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_RSSI_DIFF_CHANGED);
 	return status;
 }
 
@@ -6576,10 +6602,16 @@ QDF_STATUS sme_update_fils_config(tHalHandle hal, uint8_t session_id,
 
 	csr_update_fils_config(mac, session_id, src_profile);
 	if (csr_roamIsRoamOffloadEnabled(mac)) {
-		sme_debug("Updating fils config to fw");
-		csr_roam_offload_scan(mac, session_id,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_FILS_PARAMS_CHANGED);
+		status = sme_acquire_global_lock(&mac->sme);
+		if (QDF_IS_STATUS_SUCCESS(status)) {
+			sme_debug("Updating fils config to fw");
+			csr_roam_offload_scan(mac, session_id,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_FILS_PARAMS_CHANGED);
+			sme_release_global_lock(&mac->sme);
+		} else {
+			sme_err("Failed to acquire SME lock");
+		}
 	} else {
 		sme_info("LFR3 not enabled");
 		return QDF_STATUS_E_INVAL;
@@ -7450,13 +7482,14 @@ QDF_STATUS sme_set_neighbor_scan_refresh_period(tHalHandle hHal,
 		pNeighborRoamInfo->cfgParams.neighborResultsRefreshPeriod =
 			neighborScanResultsRefreshPeriod;
 
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+				ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+				REASON_NEIGHBOR_SCAN_REFRESH_PERIOD_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				   REASON_NEIGHBOR_SCAN_REFRESH_PERIOD_CHANGED);
-	}
+
 	return status;
 }
 
@@ -7570,13 +7603,15 @@ QDF_STATUS sme_update_empty_scan_refresh_period(tHalHandle hHal, uint8_t
 			nEmptyScanRefreshPeriod;
 		pNeighborRoamInfo->cfgParams.emptyScanRefreshPeriod =
 			nEmptyScanRefreshPeriod;
+
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+				ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+				REASON_EMPTY_SCAN_REF_PERIOD_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_EMPTY_SCAN_REF_PERIOD_CHANGED);
-	}
+
 	return status;
 }
 
@@ -7676,13 +7711,14 @@ QDF_STATUS sme_set_neighbor_scan_max_chan_time(tHalHandle hHal, uint8_t
 			nNeighborScanMaxChanTime;
 		pNeighborRoamInfo->cfgParams.maxChannelScanTime =
 			nNeighborScanMaxChanTime;
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_SCAN_CH_TIME_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_SCAN_CH_TIME_CHANGED);
-	}
+
 
 	return status;
 }
@@ -7903,13 +7939,14 @@ QDF_STATUS sme_set_neighbor_scan_period(tHalHandle hHal, uint8_t sessionId,
 			nNeighborScanPeriod;
 		pNeighborRoamInfo->cfgParams.neighborScanPeriod =
 			nNeighborScanPeriod;
+
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+					      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					      REASON_SCAN_HOME_TIME_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_SCAN_HOME_TIME_CHANGED);
-	}
 
 	return status;
 }
@@ -8033,10 +8070,7 @@ QDF_STATUS sme_change_roam_scan_channel_list(tHalHandle hHal, uint8_t sessionId,
 	pNeighborRoamInfo = &pMac->roam.neighborRoamInfo[sessionId];
 	status = sme_acquire_global_lock(&pMac->sme);
 	if (!QDF_IS_STATUS_SUCCESS(status)) {
-		if (pMac->roam.configParam.isRoamOffloadScanEnabled)
-			csr_roam_offload_scan(pMac, sessionId,
-					ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-					REASON_CHANNEL_LIST_CHANGED);
+		sme_err("Failed to acquire SME lock");
 		return status;
 	}
 	chan_info = &pNeighborRoamInfo->cfgParams.channelInfo;
@@ -8072,12 +8106,13 @@ QDF_STATUS sme_change_roam_scan_channel_list(tHalHandle hHal, uint8_t sessionId,
 		"LFR runtime successfully set roam scan channels to %s - old value is %s - roam state is %d",
 			newChannelList, oldChannelList,
 		pMac->roam.neighborRoamInfo[sessionId].neighborRoamState);
-	sme_release_global_lock(&pMac->sme);
 
 	if (pMac->roam.configParam.isRoamOffloadScanEnabled)
 		csr_roam_offload_scan(pMac, sessionId,
 				ROAM_SCAN_OFFLOAD_UPDATE_CFG,
 				REASON_CHANNEL_LIST_CHANGED);
+
+	sme_release_global_lock(&pMac->sme);
 	return status;
 }
 
@@ -10878,13 +10913,14 @@ QDF_STATUS sme_update_dfs_scan_mode(tHalHandle hHal, uint8_t sessionId,
 							    neighborRoamState));
 		pMac->roam.configParam.allowDFSChannelRoam =
 			allowDFSChannelRoam;
+		if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
+			csr_roam_offload_scan(pMac, sessionId,
+					ROAM_SCAN_OFFLOAD_UPDATE_CFG,
+					REASON_ROAM_DFS_SCAN_MODE_CHANGED);
+		}
 		sme_release_global_lock(&pMac->sme);
 	}
-	if (pMac->roam.configParam.isRoamOffloadScanEnabled) {
-		csr_roam_offload_scan(pMac, sessionId,
-				      ROAM_SCAN_OFFLOAD_UPDATE_CFG,
-				      REASON_ROAM_DFS_SCAN_MODE_CHANGED);
-	}
+
 
 	return status;
 }
@@ -13688,13 +13724,14 @@ QDF_STATUS sme_update_roam_scan_hi_rssi_scan_params(tHalHandle hal_handle,
 			status = QDF_STATUS_E_INVAL;
 			break;
 		}
+
+		if (mac_ctx->roam.configParam.isRoamOffloadScanEnabled &&
+		    status == QDF_STATUS_SUCCESS) {
+			csr_roam_offload_scan(mac_ctx, session_id,
+				ROAM_SCAN_OFFLOAD_UPDATE_CFG, reason);
+		}
 		sme_release_global_lock(&mac_ctx->sme);
 	}
-	if (mac_ctx->roam.configParam.isRoamOffloadScanEnabled &&
-		status == QDF_STATUS_SUCCESS) {
-		csr_roam_offload_scan(mac_ctx, session_id,
-			ROAM_SCAN_OFFLOAD_UPDATE_CFG, reason);
-	}
 
 	return status;
 }
@@ -14556,11 +14593,18 @@ QDF_STATUS sme_update_sta_roam_policy(tHalHandle hal_handle,
 		QDF_TRACE(QDF_MODULE_ID_SME, QDF_TRACE_LEVEL_ERROR,
 			FL("failed to update the supported channel list"));
 	}
-	if (mac_ctx->roam.configParam.isRoamOffloadScanEnabled)
-		csr_roam_offload_scan(mac_ctx, session_id,
+
+	if (mac_ctx->roam.configParam.isRoamOffloadScanEnabled) {
+		status = sme_acquire_global_lock(&mac_ctx->sme);
+		if (QDF_IS_STATUS_SUCCESS(status)) {
+			csr_roam_offload_scan(mac_ctx, session_id,
 				ROAM_SCAN_OFFLOAD_UPDATE_CFG,
 				REASON_ROAM_SCAN_STA_ROAM_POLICY_CHANGED);
-
+			sme_release_global_lock(&mac_ctx->sme);
+		} else {
+			sme_err("Failed to acquire SME lock");
+		}
+	}
 	qdf_mem_free(sme_config);
 	return status;
 }

+ 13 - 7
core/sme/src/csr/csr_neighbor_roam.c

@@ -135,14 +135,20 @@ QDF_STATUS csr_neighbor_roam_update_fast_roaming_enabled(tpAniSirGlobal mac_ctx,
 
 	switch (neighbor_roam_info->neighborRoamState) {
 	case eCSR_NEIGHBOR_ROAM_STATE_CONNECTED:
-		if (fast_roam_enabled) {
-			csr_roam_offload_scan(mac_ctx, session_id,
-					      ROAM_SCAN_OFFLOAD_START,
-					      REASON_CONNECT);
+		qdf_status = sme_acquire_global_lock(&mac_ctx->sme);
+		if (QDF_IS_STATUS_SUCCESS(qdf_status)) {
+			if (fast_roam_enabled) {
+				csr_roam_offload_scan(mac_ctx, session_id,
+						ROAM_SCAN_OFFLOAD_START,
+						REASON_CONNECT);
+			} else {
+				csr_roam_offload_scan(mac_ctx, session_id,
+						ROAM_SCAN_OFFLOAD_STOP,
+						REASON_DISCONNECTED);
+			}
+			sme_release_global_lock(&mac_ctx->sme);
 		} else {
-			csr_roam_offload_scan(mac_ctx, session_id,
-					      ROAM_SCAN_OFFLOAD_STOP,
-					      REASON_DISCONNECTED);
+			sme_err("Failed to acquire SME lock");
 		}
 	break;
 	case eCSR_NEIGHBOR_ROAM_STATE_INIT: