From ebf8a8640e2122225f96ad619978e4e227813ba3 Mon Sep 17 00:00:00 2001 From: Mohit Khanna Date: Thu, 28 Apr 2016 17:53:59 -0700 Subject: [PATCH] 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 | 51 ++++++++++++++----------------- core/cds/src/cds_api.c | 6 ++++ core/cds/src/cds_packet.c | 58 ++++++++++++++++++++++-------------- core/hdd/src/wlan_hdd_main.c | 6 +--- 4 files changed, 66 insertions(+), 55 deletions(-) diff --git a/core/cds/inc/cds_packet.h b/core/cds/inc/cds_packet.h index f63e9033345c..e85ebb3d34a6 100644 --- a/core/cds/inc/cds_packet.h +++ b/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 */ /** diff --git a/core/cds/src/cds_api.c b/core/cds/src/cds_api.c index 684bdb5dae29..12e17cfda911 100644 --- a/core/cds/src/cds_api.c +++ b/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; diff --git a/core/cds/src/cds_packet.c b/core/cds/src/cds_packet.c index 38447aeef712..f8f610459f4e 100644 --- a/core/cds/src/cds_packet.c +++ b/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 */ diff --git a/core/hdd/src/wlan_hdd_main.c b/core/hdd/src/wlan_hdd_main.c index bdfbdc3894b9..220531ca6750 100644 --- a/core/hdd/src/wlan_hdd_main.c +++ b/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);