Browse Source

qcacld-3.0: Stop bandwidth timer before adapter cleanup

There is race condition between the bus bandwidth work and cleaning up
an adapter. Under some conditions, it is possible for the bus bandwidth
work to access a paritally destroyed adapter, leading to a
use-after-free. To prevent the race condition, use the following
sequence:
    1) Stop the bandwidth timer
    2) Flush pending bandwidth work
    3) Cleanup the adapter
    4) Restart the bandwidth timer, if needed

Change-Id: I7166e75e65433d2dcb818ff8b41fe959c510a2e9
CRs-Fixed: 2025184
Dustin Brown 8 years ago
parent
commit
5ec6b5598e

+ 51 - 6
core/hdd/inc/wlan_hdd_main.h

@@ -1762,8 +1762,43 @@ hdd_wlan_get_ibss_mac_addr_from_staid(hdd_adapter_t *pAdapter,
 				      uint8_t staIdx);
 void hdd_checkandupdate_phymode(hdd_context_t *pHddCtx);
 #ifdef MSM_PLATFORM
-void hdd_start_bus_bw_compute_timer(hdd_adapter_t *pAdapter);
-void hdd_stop_bus_bw_compute_timer(hdd_adapter_t *pAdapter);
+/**
+ * hdd_bus_bw_compute_timer_start() - start the bandwidth timer
+ * @hdd_ctx: the global hdd context
+ *
+ * Return: None
+ */
+void hdd_bus_bw_compute_timer_start(hdd_context_t *hdd_ctx);
+
+/**
+ * hdd_bus_bw_compute_timer_try_start() - try to start the bandwidth timer
+ * @hdd_ctx: the global hdd context
+ *
+ * This function ensures there is at least one adapter in the associated state
+ * before starting the bandwidth timer.
+ *
+ * Return: None
+ */
+void hdd_bus_bw_compute_timer_try_start(hdd_context_t *hdd_ctx);
+
+/**
+ * hdd_bus_bw_compute_timer_stop() - stop the bandwidth timer
+ * @hdd_ctx: the global hdd context
+ *
+ * Return: None
+ */
+void hdd_bus_bw_compute_timer_stop(hdd_context_t *hdd_ctx);
+
+/**
+ * hdd_bus_bw_compute_timer_try_stop() - try to stop the bandwidth timer
+ * @hdd_ctx: the global hdd context
+ *
+ * This function ensures there are no adapters in the associated state before
+ * stopping the bandwidth timer.
+ *
+ * Return: None
+ */
+void hdd_bus_bw_compute_timer_try_stop(hdd_context_t *hdd_ctx);
 
 /**
  * hdd_bus_bandwidth_init() - Initialize bus bandwidth data structures.
@@ -1785,14 +1820,25 @@ int hdd_bus_bandwidth_init(hdd_context_t *hdd_ctx);
  */
 void hdd_bus_bandwidth_destroy(hdd_context_t *hdd_ctx);
 #else
-static inline void hdd_start_bus_bw_compute_timer(hdd_adapter_t *pAdapter)
+
+static inline void hdd_bus_bw_compute_timer_start(hdd_context_t *hdd_ctx)
+{
+}
+
+static inline void hdd_bus_bw_compute_timer_try_start(hdd_context_t *hdd_ctx)
+{
+}
+
+static inline void hdd_bus_bw_compute_timer_stop(hdd_context_t *hdd_ctx)
+{
+}
+
+static inline void hdd_bus_bw_compute_timer_try_stop(hdd_context_t *hdd_ctx)
 {
-	return;
 }
 
 static inline void hdd_stop_bus_bw_computer_timer(hdd_adapter_t *pAdapter)
 {
-	return;
 }
 
 static inline int hdd_bus_bandwidth_init(hdd_context_t *hdd_ctx)
@@ -1802,7 +1848,6 @@ static inline int hdd_bus_bandwidth_init(hdd_context_t *hdd_ctx)
 
 static inline void hdd_bus_bandwidth_destroy(hdd_context_t *hdd_ctx)
 {
-	return;
 }
 #endif
 

+ 2 - 2
core/hdd/src/wlan_hdd_assoc.c

@@ -1306,7 +1306,7 @@ static void hdd_send_association_event(struct net_device *dev,
 			&pAdapter->prev_fwd_tx_packets,
 			&pAdapter->prev_fwd_rx_packets);
 		spin_unlock_bh(&pHddCtx->bus_bw_lock);
-		hdd_start_bus_bw_compute_timer(pAdapter);
+		hdd_bus_bw_compute_timer_start(pHddCtx);
 #endif
 #endif
 	} else if (eConnectionState_IbssConnected ==    /* IBss Associated */
@@ -1371,7 +1371,7 @@ static void hdd_send_association_event(struct net_device *dev,
 		pAdapter->prev_fwd_tx_packets = 0;
 		pAdapter->prev_fwd_rx_packets = 0;
 		spin_unlock_bh(&pHddCtx->bus_bw_lock);
-		hdd_stop_bus_bw_compute_timer(pAdapter);
+		hdd_bus_bw_compute_timer_try_stop(pHddCtx);
 #endif
 	}
 	hdd_ipa_set_tx_flow_info();

+ 2 - 2
core/hdd/src/wlan_hdd_hostapd.c

@@ -1601,7 +1601,7 @@ QDF_STATUS hdd_hostapd_sap_event_cb(tpSap_Event pSapEvent,
 				&pHostapdAdapter->prev_fwd_rx_packets);
 
 			spin_unlock_bh(&pHddCtx->bus_bw_lock);
-			hdd_start_bus_bw_compute_timer(pHostapdAdapter);
+			hdd_bus_bw_compute_timer_start(pHddCtx);
 		}
 #endif
 		pHddApCtx->bApActive = true;
@@ -1815,7 +1815,7 @@ QDF_STATUS hdd_hostapd_sap_event_cb(tpSap_Event pSapEvent,
 			pHostapdAdapter->prev_fwd_tx_packets = 0;
 			pHostapdAdapter->prev_fwd_rx_packets = 0;
 			spin_unlock_bh(&pHddCtx->bus_bw_lock);
-			hdd_stop_bus_bw_compute_timer(pHostapdAdapter);
+			hdd_bus_bw_compute_timer_try_stop(pHddCtx);
 		}
 #endif
 		hdd_green_ap_del_sta(pHddCtx);

+ 116 - 53
core/hdd/src/wlan_hdd_main.c

@@ -3788,15 +3788,20 @@ QDF_STATUS hdd_close_adapter(hdd_context_t *hdd_ctx, hdd_adapter_t *adapter,
 	adapterNode = pCurrent;
 	if (QDF_STATUS_SUCCESS == status) {
 		hdd_info("wait for bus bw work to flush");
+		hdd_bus_bw_compute_timer_stop(hdd_ctx);
 		cancel_work_sync(&hdd_ctx->bus_bw_work);
+
+		/* cleanup adapter */
 		policy_mgr_clear_concurrency_mode(hdd_ctx->hdd_psoc,
 			adapter->device_mode);
 		hdd_cleanup_adapter(hdd_ctx, adapterNode->pAdapter, rtnl_held);
-
 		hdd_remove_adapter(hdd_ctx, adapterNode);
 		qdf_mem_free(adapterNode);
 		adapterNode = NULL;
 
+		/* conditionally restart the bw timer */
+		hdd_bus_bw_compute_timer_try_start(hdd_ctx);
+
 		/* Adapter removed. Decrement vdev count */
 		if (hdd_ctx->current_intf_count != 0)
 			hdd_ctx->current_intf_count--;
@@ -9668,76 +9673,134 @@ hdd_adapter_t *hdd_get_con_sap_adapter(hdd_adapter_t *this_sap_adapter,
 }
 
 #ifdef MSM_PLATFORM
-void hdd_start_bus_bw_compute_timer(hdd_adapter_t *adapter)
+static inline bool hdd_adapter_is_sta(hdd_adapter_t *adapter)
 {
-	hdd_context_t *hdd_ctx = WLAN_HDD_GET_CTX(adapter);
+	return adapter->device_mode == QDF_STA_MODE ||
+		adapter->device_mode == QDF_P2P_CLIENT_MODE;
+}
 
-	qdf_spinlock_acquire(&hdd_ctx->bus_bw_timer_lock);
-	if (!hdd_ctx->bus_bw_timer_running) {
-		hdd_ctx->bus_bw_timer_running = true;
-		qdf_timer_start(&hdd_ctx->bus_bw_timer,
-				hdd_ctx->config->busBandwidthComputeInterval);
+static inline bool hdd_adapter_is_ap(hdd_adapter_t *adapter)
+{
+	return adapter->device_mode == QDF_SAP_MODE ||
+		adapter->device_mode == QDF_P2P_GO_MODE;
+}
+
+static bool hdd_any_adapter_is_assoc(hdd_context_t *hdd_ctx)
+{
+	QDF_STATUS status;
+	hdd_adapter_list_node_t *node;
 
+	status = hdd_get_front_adapter(hdd_ctx, &node);
+	while (QDF_IS_STATUS_SUCCESS(status) && node) {
+		hdd_adapter_t *adapter = node->pAdapter;
+
+		if (adapter &&
+		    hdd_adapter_is_sta(adapter) &&
+		    WLAN_HDD_GET_STATION_CTX_PTR(adapter)->
+			conn_info.connState == eConnectionState_Associated) {
+			return true;
+		}
+
+		if (adapter &&
+		    hdd_adapter_is_ap(adapter) &&
+		    WLAN_HDD_GET_AP_CTX_PTR(adapter)->bApActive) {
+			return true;
+		}
+
+		status = hdd_get_next_adapter(hdd_ctx, node, &node);
 	}
-	qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
 
+	return false;
 }
 
-void hdd_stop_bus_bw_compute_timer(hdd_adapter_t *adapter)
+static bool hdd_bus_bw_compute_timer_is_running(hdd_context_t *hdd_ctx)
 {
-	hdd_adapter_list_node_t *adapterNode = NULL, *pNext = NULL;
-	QDF_STATUS status;
-	bool can_stop = true;
-	hdd_context_t *hdd_ctx = WLAN_HDD_GET_CTX(adapter);
+	bool is_running;
 
 	qdf_spinlock_acquire(&hdd_ctx->bus_bw_timer_lock);
-	if (!hdd_ctx->bus_bw_timer_running) {
-		qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
-		/* trying to stop timer, when not running is not good */
-		hdd_info("bus band width compute timer is not running");
+	is_running = hdd_ctx->bus_bw_timer_running;
+	qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
+
+	return is_running;
+}
+
+static void __hdd_bus_bw_compute_timer_start(hdd_context_t *hdd_ctx)
+{
+	qdf_spinlock_acquire(&hdd_ctx->bus_bw_timer_lock);
+	hdd_ctx->bus_bw_timer_running = true;
+	qdf_timer_start(&hdd_ctx->bus_bw_timer,
+			hdd_ctx->config->busBandwidthComputeInterval);
+	qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
+}
+
+void hdd_bus_bw_compute_timer_start(hdd_context_t *hdd_ctx)
+{
+	ENTER();
+
+	if (hdd_bus_bw_compute_timer_is_running(hdd_ctx)) {
+		hdd_debug("Bandwidth compute timer already started");
+		return;
+	}
+
+	__hdd_bus_bw_compute_timer_start(hdd_ctx);
+
+	EXIT();
+}
+
+void hdd_bus_bw_compute_timer_try_start(hdd_context_t *hdd_ctx)
+{
+	ENTER();
+
+	if (hdd_bus_bw_compute_timer_is_running(hdd_ctx)) {
+		hdd_debug("Bandwidth compute timer already started");
 		return;
 	}
+
+	if (hdd_any_adapter_is_assoc(hdd_ctx))
+		__hdd_bus_bw_compute_timer_start(hdd_ctx);
+
+	EXIT();
+}
+
+static void __hdd_bus_bw_compute_timer_stop(hdd_context_t *hdd_ctx)
+{
+	hdd_ipa_set_perf_level(hdd_ctx, 0, 0);
+
+	qdf_spinlock_acquire(&hdd_ctx->bus_bw_timer_lock);
+	qdf_timer_stop(&hdd_ctx->bus_bw_timer);
+	hdd_ctx->bus_bw_timer_running = false;
 	qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
 
-	if (policy_mgr_concurrent_open_sessions_running(
-		hdd_ctx->hdd_psoc)) {
-		status = hdd_get_front_adapter(hdd_ctx, &adapterNode);
+	hdd_reset_tcp_delack(hdd_ctx);
+}
 
-		while (NULL != adapterNode && QDF_STATUS_SUCCESS == status) {
-			adapter = adapterNode->pAdapter;
-			if (adapter
-			    && (adapter->device_mode == QDF_STA_MODE
-				|| adapter->device_mode == QDF_P2P_CLIENT_MODE)
-			    && WLAN_HDD_GET_STATION_CTX_PTR(adapter)->
-			    conn_info.connState ==
-			    eConnectionState_Associated) {
-				can_stop = false;
-				break;
-			}
-			if (adapter
-			    && (adapter->device_mode == QDF_SAP_MODE
-				|| adapter->device_mode == QDF_P2P_GO_MODE)
-			    && WLAN_HDD_GET_AP_CTX_PTR(adapter)->bApActive ==
-			    true) {
-				can_stop = false;
-				break;
-			}
-			status = hdd_get_next_adapter(hdd_ctx,
-						      adapterNode,
-						      &pNext);
-			adapterNode = pNext;
-		}
+void hdd_bus_bw_compute_timer_stop(hdd_context_t *hdd_ctx)
+{
+	ENTER();
+
+	if (!hdd_bus_bw_compute_timer_is_running(hdd_ctx)) {
+		hdd_debug("Bandwidth compute timer already stopped");
+		return;
 	}
 
-	if (can_stop == true) {
-		/* reset the ipa perf level */
-		hdd_ipa_set_perf_level(hdd_ctx, 0, 0);
-		qdf_spinlock_acquire(&hdd_ctx->bus_bw_timer_lock);
-		qdf_timer_stop(&hdd_ctx->bus_bw_timer);
-		hdd_ctx->bus_bw_timer_running = false;
-		qdf_spinlock_release(&hdd_ctx->bus_bw_timer_lock);
-		hdd_reset_tcp_delack(hdd_ctx);
+	__hdd_bus_bw_compute_timer_stop(hdd_ctx);
+
+	EXIT();
+}
+
+void hdd_bus_bw_compute_timer_try_stop(hdd_context_t *hdd_ctx)
+{
+	ENTER();
+
+	if (!hdd_bus_bw_compute_timer_is_running(hdd_ctx)) {
+		hdd_debug("Bandwidth compute timer already stopped");
+		return;
 	}
+
+	if (!hdd_any_adapter_is_assoc(hdd_ctx))
+		__hdd_bus_bw_compute_timer_stop(hdd_ctx);
+
+	EXIT();
 }
 #endif