WiFi-HAL: Avoid race condition in RSSI monitoring commands/events

There is a possiblility while handling the wlan driver event
in the handleEvent of the RssiMonitor, the RssiMonitor stop
is also triggered. The stop monitor cleans RssiMonitor command
instance. In this case the handleEvent will crash as the object
is already deleted. To avoid this scenario moving the handler
creation under hal_info and adding checks to ensure that handler
is enabled/disabled prior to starting and stopping the monitor
command.

BUG: 29936203
Change-Id: Id0ab73313a03510f0b15b101e381112c4ace4aef

4 files changed, 124 insertions, 42 deletions

diff --git a/qcwcn/wifi_hal/common.h b/qcwcn/wifi_hal/common.h
index f0b8373..3185b08 100644
--- a/qcwcn/wifi_hal/common.h
+++ b/qcwcn/wifi_hal/common.h
@@ -86,6 +86,7 @@ typedef struct {
 } interface_info;
 
 struct gscan_event_handlers_s;
+struct rssi_monitor_event_handler_s;
 
 typedef struct hal_info_s {
 
@@ -139,6 +140,7 @@ typedef struct hal_info_s {
     packet_fate_monitor_info *pkt_fate_stats;
     /* mutex for the packet fate stats shared resource protection */
     pthread_mutex_t pkt_fate_stats_lock;
+    struct rssi_monitor_event_handler_s *rssi_handlers;
 } hal_info;
 
 wifi_error wifi_register_handler(wifi_handle handle, int cmd, nl_recvmsg_msg_cb_t func, void *arg);
@@ -160,6 +162,8 @@ wifi_handle getWifiHandle(hal_info *info);
 wifi_interface_handle getIfaceHandle(interface_info *info);
 wifi_error initializeGscanHandlers(hal_info *info);
 wifi_error cleanupGscanHandlers(hal_info *info);
+wifi_error initializeRSSIMonitorHandler(hal_info *info);
+wifi_error cleanupRSSIMonitorHandler(hal_info *info);
 
 lowi_cb_table_t *getLowiCallbackTable(u32 requested_lowi_capabilities);
 
diff --git a/qcwcn/wifi_hal/rssi_monitor.cpp b/qcwcn/wifi_hal/rssi_monitor.cpp
index 6b96c10..8cffe98 100644
--- a/qcwcn/wifi_hal/rssi_monitor.cpp
+++ b/qcwcn/wifi_hal/rssi_monitor.cpp
@@ -38,20 +38,88 @@
 #include "rssi_monitor.h"
 #include "vendor_definitions.h"
 
-//Singleton Static Instance
-RSSIMonitorCommand* RSSIMonitorCommand::mRSSIMonitorCommandInstance = NULL;
+/* Used to handle rssi command events from driver/firmware.*/
+typedef struct rssi_monitor_event_handler_s {
+    RSSIMonitorCommand* mRSSIMonitorCommandInstance;
+} rssi_monitor_event_handlers;
+
+wifi_error initializeRSSIMonitorHandler(hal_info *info)
+{
+    info->rssi_handlers = (rssi_monitor_event_handlers *)malloc(sizeof(
+                              rssi_monitor_event_handlers));
+    if (info->rssi_handlers) {
+        memset(info->rssi_handlers, 0, sizeof(rssi_monitor_event_handlers));
+    }
+    else {
+        ALOGE("%s: Allocation of RSSI event handlers failed",
+              __FUNCTION__);
+        return WIFI_ERROR_OUT_OF_MEMORY;
+    }
+    return WIFI_SUCCESS;
+}
+
+wifi_error cleanupRSSIMonitorHandler(hal_info *info)
+{
+    rssi_monitor_event_handlers* event_handlers;
+    if (info && info->rssi_handlers) {
+        event_handlers = (rssi_monitor_event_handlers*) info->rssi_handlers;
+        if (event_handlers->mRSSIMonitorCommandInstance) {
+            delete event_handlers->mRSSIMonitorCommandInstance;
+        }
+        memset(event_handlers, 0, sizeof(rssi_monitor_event_handlers));
+        return WIFI_SUCCESS;
+    }
+    ALOGE ("%s: info or info->rssi_handlers NULL", __FUNCTION__);
+    return WIFI_ERROR_UNKNOWN;
+}
+
+void RSSIMonitorCommand::enableEventHandling()
+{
+    pthread_mutex_lock(&rm_lock);
+    mEventHandlingEnabled = true;
+    pthread_mutex_unlock(&rm_lock);
+}
+
+void RSSIMonitorCommand::disableEventHandling()
+{
+    pthread_mutex_lock(&rm_lock);
+    mEventHandlingEnabled = false;
+    pthread_mutex_unlock(&rm_lock);
+}
+
+bool RSSIMonitorCommand::isEventHandlingEnabled()
+{
+    bool eventHandlingEnabled;
+    pthread_mutex_lock(&rm_lock);
+    eventHandlingEnabled = mEventHandlingEnabled;
+    pthread_mutex_unlock(&rm_lock);
+
+    return eventHandlingEnabled;
+}
+
+void RSSIMonitorCommand::setCallbackHandler(wifi_rssi_event_handler handler)
+{
+    mHandler = handler;
+}
 
 RSSIMonitorCommand::RSSIMonitorCommand(wifi_handle handle, int id,
                                        u32 vendor_id, u32 subcmd)
         : WifiVendorCommand(handle, id, vendor_id, subcmd)
 {
-    mRSSIMonitorCommandInstance = NULL;
     memset(&mHandler, 0, sizeof(mHandler));
+    if (registerVendorHandler(vendor_id, subcmd)) {
+        /* Error case should not happen print log */
+        ALOGE("%s: Unable to register Vendor Handler Vendor Id=0x%x subcmd=%u",
+              __FUNCTION__, vendor_id, subcmd);
+    }
+    pthread_mutex_init(&rm_lock, NULL);
+    disableEventHandling();
 }
 
 RSSIMonitorCommand::~RSSIMonitorCommand()
 {
-    mRSSIMonitorCommandInstance = NULL;
+    unregisterVendorHandler(mVendor_id, mSubcmd);
+    pthread_mutex_destroy(&rm_lock);
 }
 
 void RSSIMonitorCommand::setReqId(wifi_request_id reqid)
@@ -66,10 +134,20 @@ RSSIMonitorCommand* RSSIMonitorCommand::instance(wifi_handle handle,
         ALOGE("Interface Handle is invalid");
         return NULL;
     }
+    hal_info *info = getHalInfo(handle);
+    if (!info || !info->rssi_handlers) {
+        ALOGE("rssi_handlers is invalid");
+        return NULL;
+    }
+
+    RSSIMonitorCommand* mRSSIMonitorCommandInstance =
+        info->rssi_handlers->mRSSIMonitorCommandInstance;
+
     if (mRSSIMonitorCommandInstance == NULL) {
         mRSSIMonitorCommandInstance = new RSSIMonitorCommand(handle, id,
                 OUI_QCA,
                 QCA_NL80211_VENDOR_SUBCMD_MONITOR_RSSI);
+        info->rssi_handlers->mRSSIMonitorCommandInstance = mRSSIMonitorCommandInstance;
         return mRSSIMonitorCommandInstance;
     }
     else
@@ -92,6 +170,13 @@ RSSIMonitorCommand* RSSIMonitorCommand::instance(wifi_handle handle,
 int RSSIMonitorCommand::handleEvent(WifiEvent &event)
 {
     int ret = WIFI_SUCCESS;
+
+    if (isEventHandlingEnabled() == false) {
+        ALOGE("%s: RSSI monitor isn't running or already stopped. "
+              "Nothing to do. Exit", __FUNCTION__);
+        return ret;
+    }
+
     WifiVendorCommand::handleEvent(event);
 
     /* Parse the vendordata and get the attribute */
@@ -159,29 +244,9 @@ int RSSIMonitorCommand::handleEvent(WifiEvent &event)
             ALOGE("%s: Wrong subcmd received %d", __FUNCTION__, mSubcmd);
     }
 
-    return NL_SKIP;
-}
-
-int RSSIMonitorCommand::setCallbackHandler(wifi_rssi_event_handler nHandler,
-                                           u32 event)
-{
-    int ret;
-    mHandler = nHandler;
-    ret = registerVendorHandler(mVendor_id, event);
-    if (ret != 0) {
-        /* Error case should not happen print log */
-        ALOGE("%s: Unable to register Vendor Handler Vendor Id=0x%x subcmd=%u",
-              __FUNCTION__, mVendor_id, mSubcmd);
-    }
     return ret;
 }
 
-wifi_error RSSIMonitorCommand::unregisterHandler(u32 subCmd)
-{
-    unregisterVendorHandler(mVendor_id, subCmd);
-    return WIFI_SUCCESS;
-}
-
 wifi_error wifi_start_rssi_monitoring(wifi_request_id id,
                                       wifi_interface_handle iface,
                                       s8 max_rssi,
@@ -233,15 +298,14 @@ wifi_error wifi_start_rssi_monitoring(wifi_request_id id,
         return WIFI_ERROR_OUT_OF_MEMORY;
     }
 
-    ret = rssiCommand->setCallbackHandler(eh,
-            QCA_NL80211_VENDOR_SUBCMD_MONITOR_RSSI);
-    if (ret < 0)
-        goto cleanup;
+    rssiCommand->setCallbackHandler(eh);
 
     ret = vCommand->requestResponse();
     if (ret < 0)
         goto cleanup;
 
+    rssiCommand->enableEventHandling();
+
 cleanup:
     delete vCommand;
     return (wifi_error)ret;
@@ -255,6 +319,18 @@ wifi_error wifi_stop_rssi_monitoring(wifi_request_id id,
     WifiVendorCommand *vCommand = NULL;
     wifi_handle wifiHandle = getWifiHandle(iface);
     RSSIMonitorCommand *rssiCommand;
+    rssi_monitor_event_handlers* event_handlers;
+    hal_info *info = getHalInfo(wifiHandle);
+
+    event_handlers = (rssi_monitor_event_handlers*)info->rssi_handlers;
+    rssiCommand = event_handlers->mRSSIMonitorCommandInstance;
+
+    if (rssiCommand == NULL ||
+        rssiCommand->isEventHandlingEnabled() == false) {
+        ALOGE("%s: RSSI monitor isn't running or already stopped. "
+            "Nothing to do. Exit", __FUNCTION__);
+        return WIFI_ERROR_NOT_AVAILABLE;
+    }
 
     ret = initialize_vendor_cmd(iface, id,
                                 QCA_NL80211_VENDOR_SUBCMD_MONITOR_RSSI,
@@ -285,19 +361,8 @@ wifi_error wifi_stop_rssi_monitoring(wifi_request_id id,
     if (ret < 0)
         goto cleanup;
 
-    rssiCommand = RSSIMonitorCommand::instance(wifiHandle, id);
-    if (rssiCommand == NULL) {
-        ALOGE("%s: Error rssiCommand NULL", __FUNCTION__);
-        ret = WIFI_ERROR_OUT_OF_MEMORY;
-        goto cleanup;
-    }
-
-    ret = rssiCommand->unregisterHandler(
-                                        QCA_NL80211_VENDOR_SUBCMD_MONITOR_RSSI);
-    if (ret != WIFI_SUCCESS)
-        goto cleanup;
+    rssiCommand->disableEventHandling();
 
-    delete rssiCommand;
 
 cleanup:
     delete vCommand;
diff --git a/qcwcn/wifi_hal/rssi_monitor.h b/qcwcn/wifi_hal/rssi_monitor.h
index bd3d88e..c6ea692 100644
--- a/qcwcn/wifi_hal/rssi_monitor.h
+++ b/qcwcn/wifi_hal/rssi_monitor.h
@@ -42,14 +42,19 @@ private:
     static RSSIMonitorCommand *mRSSIMonitorCommandInstance;
     wifi_rssi_event_handler mHandler;
     RSSIMonitorCommand(wifi_handle handle, int id, u32 vendor_id, u32 subcmd);
+    bool mEventHandlingEnabled;
+    /* mutex for the mEventHandlingEnabled access*/
+    pthread_mutex_t rm_lock;
 
 public:
     virtual ~RSSIMonitorCommand();
     static RSSIMonitorCommand* instance(wifi_handle handle, wifi_request_id id);
-    virtual int setCallbackHandler(wifi_rssi_event_handler nHandler, u32 event);
     virtual int handleEvent(WifiEvent &event);
-    virtual wifi_error unregisterHandler(u32 subCmd);
     virtual void setReqId(wifi_request_id reqid);
+    virtual void setCallbackHandler(wifi_rssi_event_handler nHandler);
+    void enableEventHandling();
+    void disableEventHandling();
+    bool isEventHandlingEnabled();
 };
 
 #ifdef __cplusplus
diff --git a/qcwcn/wifi_hal/wifi_hal.cpp b/qcwcn/wifi_hal/wifi_hal.cpp
index d59d138..2f78718 100644
--- a/qcwcn/wifi_hal/wifi_hal.cpp
+++ b/qcwcn/wifi_hal/wifi_hal.cpp
@@ -560,6 +560,12 @@ wifi_error wifi_initialize(wifi_handle *handle)
         goto unload;
     }
 
+    ret = initializeRSSIMonitorHandler(info);
+    if (ret != WIFI_SUCCESS) {
+        ALOGE("Initializing RSSI Event Handler Failed");
+        goto unload;
+    }
+
     ALOGV("Initialized Wifi HAL Successfully; vendor cmd = %d Supported"
             " features : %x", NL80211_CMD_VENDOR, info->supported_feature_set);
 
@@ -576,6 +582,7 @@ unload:
             if (info->pkt_stats) free(info->pkt_stats);
             if (info->rx_aggr_pkts) free(info->rx_aggr_pkts);
             cleanupGscanHandlers(info);
+            cleanupRSSIMonitorHandler(info);
             free(info);
         }
     }
@@ -626,6 +633,7 @@ static void internal_cleaned_up_handler(wifi_handle handle)
         free(info->rx_aggr_pkts);
     wifi_logger_ring_buffers_deinit(info);
     cleanupGscanHandlers(info);
+    cleanupRSSIMonitorHandler(info);
 
     if (info->exit_sockets[0] >= 0) {
         close(info->exit_sockets[0]);