qcacld-3.0: Fix use-after-free crash for trace buffer

There is a race condition between hdd_disconnect_handler and
cds_pkt_proto_trace_close while trying to update the trace_buffer.

Fix race condition by calling cds_pkt_proto_trace_deinit (formerly
cds_pkt_proto_trace_close) at the end of cds_close, after all the threads
have been torn down and other modules have been shut. Similarly,
call cds_pkt_proto_trace_init at the begining of cds_open, so it gets
initialized before any of the threads are up or any modules have been
initialized.

Change-Id: If9e2d6a360d814b7f1b0c92789dc0fd3fe3d974b
CRs-Fixed: 1009162

core/cds/inc/cds_packet.h

@@ -76,42 +76,37 @@ uint8_t cds_pkt_get_proto_type
 	(struct sk_buff *skb, uint8_t tracking_map, uint8_t dot11_type);
 
 #ifdef QCA_PKT_PROTO_TRACE
-/*---------------------------------------------------------------------------
-
-* brief cds_pkt_trace_buf_update() -
-      Update storage buffer with interest event string
-
-* event_string Event String may packet type or outstanding event
-
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_trace_buf_update() - Update storage buffer with interest event string
+ * @event_string: string may be a packet type or an outstanding event
+ *
+ * Return: none
+ */
 void cds_pkt_trace_buf_update(char *event_string);
 
-/*---------------------------------------------------------------------------
-
-* brief cds_pkt_trace_buf_dump() -
-      Dump stored information into kernel log
-
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_trace_buf_dump() - Dump stored information into kernel log
+ *
+ * Return: none
+ */
 void cds_pkt_trace_buf_dump(void);
 
-/*---------------------------------------------------------------------------
-
-* brief cds_pkt_proto_trace_init() -
-      Initialize protocol trace functionality, allocate required resource
-
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_proto_trace_init() - Initialize protocol trace functionality
+ *
+ * Return: none
+ */
 void cds_pkt_proto_trace_init(void);
 
-/*---------------------------------------------------------------------------
-
-* brief cds_pkt_proto_trace_close() -
-      Free required resource
-
-   ---------------------------------------------------------------------------*/
-void cds_pkt_proto_trace_close(void);
+/**
+ * cds_pkt_proto_trace_deinit() - Free protocol trace buffer resource
+ *
+ * Return: none
+ */
+void cds_pkt_proto_trace_deinit(void);
 #else
 static inline void cds_pkt_proto_trace_init(void) { }
-static inline void cds_pkt_proto_trace_close(void) {}
+static inline void cds_pkt_proto_trace_deinit(void) {}
 #endif /* QCA_PKT_PROTO_TRACE */
 
 /**

core/cds/src/cds_api.c

@@ -223,6 +223,9 @@ QDF_STATUS cds_open(void)
 	/* Initialize bug reporting structure */
 	cds_init_log_completion();
 
+	/* Initialize protocol trace functionality */
+	cds_pkt_proto_trace_init();
+
 	/* Initialize the probe event */
 	if (qdf_event_create(&gp_cds_context->ProbeEvent) != QDF_STATUS_SUCCESS) {
 		QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_FATAL,
@@ -853,6 +856,9 @@ QDF_STATUS cds_close(v_CONTEXT_t cds_context)
 		QDF_ASSERT(QDF_IS_STATUS_SUCCESS(qdf_status));
 	}
 
+	/* De-Initialize protocol trace functionality */
+	cds_pkt_proto_trace_deinit();
+
 	cds_deinit_log_completion();
 
 	gp_cds_context->pHDDContext = NULL;

core/cds/src/cds_packet.c

@@ -181,12 +181,12 @@ uint8_t cds_pkt_get_proto_type(struct sk_buff *skb, uint8_t tracking_map,
 }
 
 #ifdef QCA_PKT_PROTO_TRACE
-/*---------------------------------------------------------------------------
-* @brief cds_pkt_trace_buf_update() -
-      Update storage buffer with interest event string
-
-* event_string Event String may packet type or outstanding event
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_trace_buf_update() - Update storage buffer with interest event string
+ * @event_string: string may be a packet type or an outstanding event
+ *
+ * Return: none
+ */
 void cds_pkt_trace_buf_update(char *event_string)
 {
 	uint32_t slot;
@@ -194,6 +194,11 @@ void cds_pkt_trace_buf_update(char *event_string)
 	QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_INFO,
 		  "%s %d, %s", __func__, __LINE__, event_string);
 	qdf_spinlock_acquire(&trace_buffer_lock);
+	if (!trace_buffer) {
+		QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR,
+			"trace_buffer is null");
+		goto release_lock;
+	}
 	slot = trace_buffer_order % CDS_PKT_TRAC_MAX_TRACE_BUF;
 	trace_buffer[slot].order = trace_buffer_order;
 	trace_buffer[slot].event_time = qdf_mc_timer_get_system_time();
@@ -204,20 +209,27 @@ void cds_pkt_trace_buf_update(char *event_string)
 		     (CDS_PKT_TRAC_MAX_STRING_LEN < strlen(event_string)) ?
 		     CDS_PKT_TRAC_MAX_STRING_LEN : strlen(event_string));
 	trace_buffer_order++;
-	qdf_spinlock_release(&trace_buffer_lock);
 
+release_lock:
+	qdf_spinlock_release(&trace_buffer_lock);
 	return;
 }
 
-/*---------------------------------------------------------------------------
-* @brief cds_pkt_trace_buf_dump() -
-      Dump stored information into kernel log
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_trace_buf_dump() - Dump stored information into kernel log
+ *
+ * Return: none
+ */
 void cds_pkt_trace_buf_dump(void)
 {
 	uint32_t slot, idx;
 
 	qdf_spinlock_acquire(&trace_buffer_lock);
+	if (!trace_buffer) {
+		QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR,
+			"trace_buffer is null");
+		goto release_lock;
+	}
 	QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR,
 		  "PACKET TRACE DUMP START Current Timestamp %u",
 		  (unsigned int)qdf_mc_timer_get_system_time());
@@ -245,15 +257,16 @@ void cds_pkt_trace_buf_dump(void)
 
 	QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR,
 		  "PACKET TRACE DUMP END");
+release_lock:
 	qdf_spinlock_release(&trace_buffer_lock);
-
 	return;
 }
 
-/*---------------------------------------------------------------------------
-* @brief cds_pkt_proto_trace_init() -
-      Initialize protocol trace functionality, allocate required resource
-   ---------------------------------------------------------------------------*/
+/**
+ * cds_pkt_proto_trace_init() - Initialize protocol trace functionality
+ *
+ * Return: none
+ */
 void cds_pkt_proto_trace_init(void)
 {
 	/* Init spin lock to protect global memory */
@@ -269,17 +282,18 @@ void cds_pkt_proto_trace_init(void)
 	return;
 }
 
-/*---------------------------------------------------------------------------
-* @brief cds_pkt_proto_trace_close() -
-      Free required resource
-   ---------------------------------------------------------------------------*/
-void cds_pkt_proto_trace_close(void)
+/**
+ * cds_pkt_proto_trace_deinit() - Free trace_buffer resource
+ *
+ * Return: none
+ */
+void cds_pkt_proto_trace_deinit(void)
 {
 	QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR,
 		  "%s %d", __func__, __LINE__);
 	qdf_mem_free(trace_buffer);
+	trace_buffer = NULL;
 	qdf_spinlock_destroy(&trace_buffer_lock);
-
 	return;
 }
 #endif /* QCA_PKT_PROTO_TRACE */

core/hdd/src/wlan_hdd_main.c

@@ -4036,11 +4036,9 @@ void __hdd_wlan_exit(void)
 
 	memdump_deinit();
 
-#ifdef QCA_PKT_PROTO_TRACE
-	cds_pkt_proto_trace_close();
-#endif
 	/* Do all the cleanup before deregistering the driver */
 	hdd_wlan_exit(hdd_ctx);
+
 	EXIT();
 }
 
@@ -5863,8 +5861,6 @@ int hdd_wlan_startup(struct device *dev, void *hif_sc)
 		goto err_cds_disable;
 	}
 
-	cds_pkt_proto_trace_init();
-
 	rtnl_held = hdd_hold_rtnl_lock();
 
 	adapter = hdd_open_interfaces(hdd_ctx, rtnl_held);