From 887edeb65e49bca50d00d4c3c5c6d44c0a0e5b41 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Dhanotiya Date: Mon, 24 May 2021 17:46:07 +0530 Subject: [PATCH] qcacld-3.0: Move call to hdd power stats callback inside lock Currently when host receives power stats from fw, before processing these stats it check for the hdd callback and processes these stats only if hdd callback is registered. This callback is checked in wma_unified_power_debug_stats_event_handler function before processing of the fw event and outside of any lock. After processing stats, this hdd cb is invoked, there is an issue with this approach that while processing these stats if timeout happens in hdd then hdd sets its callback as NULL and now after processing stats wma handler invokes this hdd cb which is already NULL, leading to null pointer dereference. To avoid above issue, add sme callback and move null check for hdd cb and call to hdd cb inside sme lock which is the same lock used while setting hdd cb to null on timeout. Change-Id: I2203083a17ca3404c5c5af98ddc48ba127e6aee2 CRs-Fixed: 2950633 --- core/sme/inc/sme_internal.h | 3 ++ core/sme/src/common/sme_api.c | 56 +++++++++++++++++++++++++++++++++++ core/wma/src/wma_features.c | 13 ++++---- 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/core/sme/inc/sme_internal.h b/core/sme/inc/sme_internal.h index 6750a8b94aa0..48ee84f38122 100644 --- a/core/sme/inc/sme_internal.h +++ b/core/sme/inc/sme_internal.h @@ -392,6 +392,9 @@ struct sme_context { void *power_debug_stats_context; void (*power_stats_resp_callback)(struct power_stats_response *rsp, void *callback_context); + void (*sme_power_debug_stats_callback)( + struct mac_context *mac, + struct power_stats_response *response); #endif #ifdef WLAN_FEATURE_BEACON_RECEPTION_STATS void *beacon_stats_context; diff --git a/core/sme/src/common/sme_api.c b/core/sme/src/common/sme_api.c index 03252fdb4cf6..e86af82d6a7b 100644 --- a/core/sme/src/common/sme_api.c +++ b/core/sme/src/common/sme_api.c @@ -733,6 +733,59 @@ static void sme_register_debug_callback(void) } #endif /* WLAN_FEATURE_MEMDUMP_ENABLE */ +#ifdef WLAN_POWER_DEBUG +static void sme_power_debug_stats_cb(struct mac_context *mac, + struct power_stats_response *response) +{ + QDF_STATUS status = QDF_STATUS_SUCCESS; + + status = sme_acquire_global_lock(&mac->sme); + if (QDF_IS_STATUS_SUCCESS(status)) { + if (mac->sme.power_stats_resp_callback) + mac->sme.power_stats_resp_callback( + response, + mac->sme.power_debug_stats_context); + else + sme_err("Null hdd cb"); + mac->sme.power_stats_resp_callback = NULL; + mac->sme.power_debug_stats_context = NULL; + sme_release_global_lock(&mac->sme); + } +} + +static void sme_register_power_debug_stats_cb(struct mac_context *mac) +{ + QDF_STATUS status = QDF_STATUS_SUCCESS; + + status = sme_acquire_global_lock(&mac->sme); + + if (QDF_IS_STATUS_SUCCESS(status)) { + mac->sme.sme_power_debug_stats_callback = + sme_power_debug_stats_cb; + sme_release_global_lock(&mac->sme); + } +} + +static void sme_unregister_power_debug_stats_cb(struct mac_context *mac) +{ + QDF_STATUS status = QDF_STATUS_SUCCESS; + + status = sme_acquire_global_lock(&mac->sme); + if (QDF_IS_STATUS_SUCCESS(status)) { + mac->sme.sme_power_debug_stats_callback = NULL; + sme_release_global_lock(&mac->sme); + } +} +#else +static inline void sme_register_power_debug_stats_cb(struct mac_context *mac) +{ +} + +static inline void sme_unregister_power_debug_stats_cb(struct mac_context *mac) +{ +} +#endif + /* Global APIs */ /** @@ -789,6 +842,7 @@ QDF_STATUS sme_open(mac_handle_t mac_handle) } sme_trace_init(mac); sme_register_debug_callback(); + sme_register_power_debug_stats_cb(mac); return status; } @@ -3178,6 +3232,8 @@ QDF_STATUS sme_close(mac_handle_t mac_handle) if (!mac) return QDF_STATUS_E_FAILURE; + sme_unregister_power_debug_stats_cb(mac); + status = csr_close(mac); if (!QDF_IS_STATUS_SUCCESS(status)) { sme_err("csr_close failed with status: %d", status); diff --git a/core/wma/src/wma_features.c b/core/wma/src/wma_features.c index 9a60e5dd0f40..d8d9032c987a 100644 --- a/core/wma/src/wma_features.c +++ b/core/wma/src/wma_features.c @@ -4556,8 +4556,8 @@ int wma_unified_power_debug_stats_event_handler(void *handle, param_buf = (wmi_pdev_chip_power_stats_event_fixed_param *) param_tlvs->fixed_param; - if (!mac || !mac->sme.power_stats_resp_callback) { - wma_debug("NULL mac ptr or HDD callback is null"); + if (!mac) { + wma_debug("NULL mac ptr"); return -EINVAL; } @@ -4608,12 +4608,9 @@ int wma_unified_power_debug_stats_event_handler(void *handle, qdf_mem_copy(power_stats_results->debug_registers, debug_registers, stats_registers_len); - - mac->sme.power_stats_resp_callback(power_stats_results, - mac->sme.power_debug_stats_context); - - mac->sme.power_stats_resp_callback = NULL; - mac->sme.power_debug_stats_context = NULL; + if (mac->sme.sme_power_debug_stats_callback) + mac->sme.sme_power_debug_stats_callback(mac, + power_stats_results); qdf_mem_free(power_stats_results); return 0;