Add mutual exclusion to wifi HAL infrastructure

This change restricts free access from multiple threads to event buffers.
Mutual exclusion is added to avoid crashes and corruption that may
happen because of simultaneous access from a command and an event.

Access to commands from multiple threads is still not protected. But
thankfully, framework takes care of it.

Bug: 15188917
Change-Id: I41e294f01a85097e30078f603b113da9a319ba83

diff --git a/bcmdhd/wifi_hal/common.cpp b/bcmdhd/wifi_hal/common.cpp
index d5bd854..421557b 100644 (file)
--- a/bcmdhd/wifi_hal/common.cpp
+++ b/bcmdhd/wifi_hal/common.cpp
@@ -42,6 +42,9 @@ wifi_error wifi_register_handler(wifi_handle handle, int cmd, nl_recvmsg_msg_cb_
     hal_info *info = (hal_info *)handle;
 
     /* TODO: check for multiple handlers? */
+    pthread_mutex_lock(&info->cb_lock);
+
+    wifi_error result = WIFI_ERROR_OUT_OF_MEMORY;
 
     if (info->num_event_cb < info->alloc_event_cb) {
         info->event_cb[info->num_event_cb].nl_cmd  = cmd;
@@ -51,10 +54,11 @@ wifi_error wifi_register_handler(wifi_handle handle, int cmd, nl_recvmsg_msg_cb_
         info->event_cb[info->num_event_cb].cb_arg  = arg;
         info->num_event_cb++;
         ALOGI("Successfully added event handler %p for command %d", func, cmd);
-        return WIFI_SUCCESS;
-    } else {
-        return WIFI_ERROR_OUT_OF_MEMORY;
+        result = WIFI_SUCCESS;
     }
+
+    pthread_mutex_unlock(&info->cb_lock);
+    return result;
 }
 
 wifi_error wifi_register_vendor_handler(wifi_handle handle,
@@ -63,6 +67,9 @@ wifi_error wifi_register_vendor_handler(wifi_handle handle,
     hal_info *info = (hal_info *)handle;
 
     /* TODO: check for multiple handlers? */
+    pthread_mutex_lock(&info->cb_lock);
+
+    wifi_error result = WIFI_ERROR_OUT_OF_MEMORY;
 
     if (info->num_event_cb < info->alloc_event_cb) {
         info->event_cb[info->num_event_cb].nl_cmd  = NL80211_CMD_VENDOR;
@@ -72,10 +79,11 @@ wifi_error wifi_register_vendor_handler(wifi_handle handle,
         info->event_cb[info->num_event_cb].cb_arg  = arg;
         info->num_event_cb++;
         ALOGI("Added event handler %p for vendor 0x%0x and subcmd 0x%0x", func, id, subcmd);
-        return WIFI_SUCCESS;
-    } else {
-        return WIFI_ERROR_OUT_OF_MEMORY;
+        result = WIFI_SUCCESS;
     }
+
+    pthread_mutex_unlock(&info->cb_lock);
+    return result;
 }
 
 void wifi_unregister_handler(wifi_handle handle, int cmd)
@@ -84,23 +92,30 @@ void wifi_unregister_handler(wifi_handle handle, int cmd)
 
     if (cmd == NL80211_CMD_VENDOR) {
         ALOGE("Must use wifi_unregister_vendor_handler to remove vendor handlers");
+        return;
     }
 
+    pthread_mutex_lock(&info->cb_lock);
+
     for (int i = 0; i < info->num_event_cb; i++) {
         if (info->event_cb[i].nl_cmd == cmd) {
             memmove(&info->event_cb[i], &info->event_cb[i+1],
                 (info->num_event_cb - i) * sizeof(cb_info));
             info->num_event_cb--;
             ALOGI("Successfully removed event handler for command %d", cmd);
-            return;
+            break;
         }
     }
+
+    pthread_mutex_unlock(&info->cb_lock);
 }
 
 void wifi_unregister_vendor_handler(wifi_handle handle, uint32_t id, int subcmd)
 {
     hal_info *info = (hal_info *)handle;
 
+    pthread_mutex_lock(&info->cb_lock);
+
     for (int i = 0; i < info->num_event_cb; i++) {
 
         if (info->event_cb[i].nl_cmd == NL80211_CMD_VENDOR
@@ -111,9 +126,11 @@ void wifi_unregister_vendor_handler(wifi_handle handle, uint32_t id, int subcmd)
                 (info->num_event_cb - i) * sizeof(cb_info));
             info->num_event_cb--;
             ALOGI("Successfully removed event handler for vendor 0x%0x", id);
-            return;
+            break;
         }
     }
+
+    pthread_mutex_unlock(&info->cb_lock);
 }
 
 
@@ -123,15 +140,17 @@ wifi_error wifi_register_cmd(wifi_handle handle, int id, WifiCommand *cmd)
 
     ALOGD("registering command %d", id);
 
+    wifi_error result = WIFI_ERROR_OUT_OF_MEMORY;
+
     if (info->num_cmd < info->alloc_cmd) {
         info->cmd[info->num_cmd].id   = id;
         info->cmd[info->num_cmd].cmd  = cmd;
         info->num_cmd++;
         ALOGI("Successfully added command %d: %p", id, cmd);
-        return WIFI_SUCCESS;
-    } else {
-        return WIFI_ERROR_OUT_OF_MEMORY;
+        result = WIFI_SUCCESS;
     }
+
+    return result;
 }
 
 WifiCommand *wifi_unregister_cmd(wifi_handle handle, int id)
@@ -140,17 +159,19 @@ WifiCommand *wifi_unregister_cmd(wifi_handle handle, int id)
 
     ALOGD("un-registering command %d", id);
 
+    WifiCommand *cmd = NULL;
+
     for (int i = 0; i < info->num_cmd; i++) {
         if (info->cmd[i].id == id) {
-            WifiCommand *cmd = info->cmd[i].cmd;
+            cmd = info->cmd[i].cmd;
             memmove(&info->cmd[i], &info->cmd[i+1], (info->num_cmd - i) * sizeof(cmd_info));
             info->num_cmd--;
             ALOGI("Successfully removed command %d: %p", id, cmd);
-            return cmd;
+            break;
         }
     }
 
-    return NULL;
+    return cmd;
 }
 
 void wifi_unregister_cmd(wifi_handle handle, WifiCommand *cmd)
@@ -163,8 +184,7 @@ void wifi_unregister_cmd(wifi_handle handle, WifiCommand *cmd)
             memmove(&info->cmd[i], &info->cmd[i+1], (info->num_cmd - i) * sizeof(cmd_info));
             info->num_cmd--;
             ALOGI("Successfully removed command %d: %p", id, cmd);
-            return;
+            break;
         }
     }
 }
-

diff --git a/bcmdhd/wifi_hal/common.h b/bcmdhd/wifi_hal/common.h
index cdd87fb..a73c173 100644 (file)
--- a/bcmdhd/wifi_hal/common.h
+++ b/bcmdhd/wifi_hal/common.h
@@ -8,6 +8,7 @@
 
 #include <utils/Log.h>
 #include "nl80211_copy.h"
+#include "sync.h"
 
 #define SOCKET_BUFFER_SIZE      (32768U)
 #define RECV_BUF_SIZE           (4096)
@@ -95,6 +96,7 @@ typedef struct {
     cb_info *event_cb;                              // event callbacks
     int num_event_cb;                               // number of event callbacks
     int alloc_event_cb;                             // number of allocated callback objects
+    pthread_mutex_t cb_lock;                        // mutex for the event_cb access
 
     cmd_info *cmd;                                  // Outstanding commands
     int num_cmd;                                    // number of commands
@@ -103,6 +105,7 @@ typedef struct {
     interface_info **interfaces;                    // array of interfaces
     int num_interfaces;                             // number of interfaces
 
+
     // add other details
 } hal_info;
 

diff --git a/bcmdhd/wifi_hal/wifi_hal.cpp b/bcmdhd/wifi_hal/wifi_hal.cpp
index 9680344..640c5e2 100644 (file)
--- a/bcmdhd/wifi_hal/wifi_hal.cpp
+++ b/bcmdhd/wifi_hal/wifi_hal.cpp
@@ -150,6 +150,8 @@ wifi_error wifi_initialize(wifi_handle *handle)
                return WIFI_ERROR_UNKNOWN;
     }
 
+    pthread_mutex_init(&info->cb_lock, NULL);
+
     *handle = (wifi_handle) info;
 
     wifi_add_membership(*handle, "scan");
@@ -160,6 +162,7 @@ wifi_error wifi_initialize(wifi_handle *handle)
     wifi_init_interfaces(*handle);
     // ALOGI("Found %d interfaces", info->num_interfaces);
 
+
     ALOGI("Initialized Wifi HAL Successfully; vendor cmd = %d", NL80211_CMD_VENDOR);
     return WIFI_SUCCESS;
 }
@@ -196,6 +199,7 @@ static void internal_cleaned_up_handler(wifi_handle handle)
     }
 
     (*cleaned_up_handler)(handle);
+    pthread_mutex_destroy(&info->cb_lock);
     free(info);
 
     ALOGI("Internal cleanup completed");
@@ -305,6 +309,9 @@ static int internal_valid_message_handler(nl_msg *msg, void *arg)
     // event.log();
 
     bool dispatched = false;
+
+    pthread_mutex_lock(&info->cb_lock);
+
     for (int i = 0; i < info->num_event_cb; i++) {
         if (cmd == info->event_cb[i].nl_cmd) {
             if (cmd == NL80211_CMD_VENDOR
@@ -322,9 +329,10 @@ static int internal_valid_message_handler(nl_msg *msg, void *arg)
     }
 
     if (!dispatched) {
-        ALOGI("event ignored!!");
+        // ALOGI("event ignored!!");
     }
 
+    pthread_mutex_unlock(&info->cb_lock);
     return NL_OK;
 }