From da3607d6f5715022c53f52b7ed700be0fdc66e46 Mon Sep 17 00:00:00 2001
From: Vinit Deshpande <vinitd@google.com>
Date: Thu, 4 Jun 2015 20:01:03 -0700
Subject: Fix a deadlock in wifi_cleanup

pthread_mutex isn't re-entrant; and results in a hang
if called second time from the same thread. This change
ensures that it is locked only once.

Bug: 21627368
Change-Id: I9c28c1df240316c2a7eafdefa990b9582bc05a9a
---
 bcmdhd/wifi_hal/wifi_hal.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bcmdhd/wifi_hal/wifi_hal.cpp b/bcmdhd/wifi_hal/wifi_hal.cpp
index 03bcc4b..20d2b28 100644
--- a/bcmdhd/wifi_hal/wifi_hal.cpp
+++ b/bcmdhd/wifi_hal/wifi_hal.cpp
@@ -283,7 +283,9 @@ void wifi_cleanup(wifi_handle handle, wifi_cleaned_up_handler handler)
         WifiCommand *cmd = (WifiCommand *)cbi->cb_arg;
         if (cmd != NULL) {
             cmd->addRef();
+            pthread_mutex_unlock(&info->cb_lock);
             cmd->cancel();
+            pthread_mutex_lock(&info->cb_lock);
             cmd->releaseRef();
         }
     }
-- 
cgit v1.1


From c6e2679d2701404570f4f164e09dbdab92a1dafe Mon Sep 17 00:00:00 2001
From: Vinit Deshpande <vinitd@google.com>
Date: Tue, 7 Jul 2015 14:52:54 -0700
Subject: Fix incorrect wifi_cleanup

The cleanup needs to cancel commands, not the event handlers;
and needs a while loop instead of a for loop.

Also added command names to identify which commands are causing
problems.

Bug: 22302517
Change-Id: I93c3e51f7cd3ad62c1a34eeb5708d9552e37342b
---
 bcmdhd/wifi_hal/cpp_bindings.h       | 14 ++++++++++----
 bcmdhd/wifi_hal/gscan.cpp            | 34 ++++++++++++++++++----------------
 bcmdhd/wifi_hal/link_layer_stats.cpp |  2 +-
 bcmdhd/wifi_hal/rtt.cpp              |  6 +++---
 bcmdhd/wifi_hal/wifi_hal.cpp         | 35 +++++++++++++++++++++++++----------
 bcmdhd/wifi_hal/wifi_logger.cpp      | 20 +++++++++++---------
 bcmdhd/wifi_hal/wifi_offload.cpp     |  8 ++++----
 7 files changed, 72 insertions(+), 47 deletions(-)

diff --git a/bcmdhd/wifi_hal/cpp_bindings.h b/bcmdhd/wifi_hal/cpp_bindings.h
index 491c398..250976e 100644
--- a/bcmdhd/wifi_hal/cpp_bindings.h
+++ b/bcmdhd/wifi_hal/cpp_bindings.h
@@ -213,6 +213,7 @@ private:
 class WifiCommand
 {
 protected:
+    const char *mType;
     hal_info *mInfo;
     WifiRequest mMsg;
     Condition mCondition;
@@ -220,16 +221,17 @@ protected:
     interface_info *mIfaceInfo;
     int mRefs;
 public:
-    WifiCommand(wifi_handle handle, wifi_request_id id)
-            : mMsg(getHalInfo(handle)->nl80211_family_id), mId(id), mRefs(1)
+    WifiCommand(const char *type, wifi_handle handle, wifi_request_id id)
+            : mType(type), mMsg(getHalInfo(handle)->nl80211_family_id), mId(id), mRefs(1)
     {
         mIfaceInfo = NULL;
         mInfo = getHalInfo(handle);
         // ALOGD("WifiCommand %p created, mInfo = %p, mIfaceInfo = %p", this, mInfo, mIfaceInfo);
     }
 
-    WifiCommand(wifi_interface_handle iface, wifi_request_id id)
-            : mMsg(getHalInfo(iface)->nl80211_family_id, getIfaceInfo(iface)->id), mId(id), mRefs(1)
+    WifiCommand(const char *type, wifi_interface_handle iface, wifi_request_id id)
+            : mType(type), mMsg(getHalInfo(iface)->nl80211_family_id, getIfaceInfo(iface)->id),
+            mId(id), mRefs(1)
     {
         mIfaceInfo = getIfaceInfo(iface);
         mInfo = getHalInfo(iface);
@@ -244,6 +246,10 @@ public:
         return mId;
     }
 
+    const char *getType() {
+        return mType;
+    }
+
     virtual void addRef() {
         int refs = __sync_add_and_fetch(&mRefs, 1);
         // ALOGD("addRef: WifiCommand %p has %d references", this, refs);
diff --git a/bcmdhd/wifi_hal/gscan.cpp b/bcmdhd/wifi_hal/gscan.cpp
index f919150..7edeb6c 100644
--- a/bcmdhd/wifi_hal/gscan.cpp
+++ b/bcmdhd/wifi_hal/gscan.cpp
@@ -150,7 +150,7 @@ class GetCapabilitiesCommand : public WifiCommand
     wifi_gscan_capabilities *mCapabilities;
 public:
     GetCapabilitiesCommand(wifi_interface_handle iface, wifi_gscan_capabilities *capabitlites)
-        : WifiCommand(iface, 0), mCapabilities(capabitlites)
+        : WifiCommand("GetGscanCapabilitiesCommand", iface, 0), mCapabilities(capabitlites)
     {
         memset(mCapabilities, 0, sizeof(*mCapabilities));
     }
@@ -208,8 +208,8 @@ class GetChannelListCommand : public WifiCommand
 public:
     GetChannelListCommand(wifi_interface_handle iface, wifi_channel *channel_buf, int *ch_num,
         int num_max_ch, int band)
-        : WifiCommand(iface, 0), channels(channel_buf), max_channels(num_max_ch), num_channels(ch_num),
-        band(band)
+        : WifiCommand("GetChannelListCommand", iface, 0), channels(channel_buf),
+            max_channels(num_max_ch), num_channels(ch_num), band(band)
     {
         memset(channels, 0, sizeof(wifi_channel) * max_channels);
     }
@@ -351,7 +351,7 @@ class FullScanResultsCommand : public WifiCommand
 public:
     FullScanResultsCommand(wifi_interface_handle iface, int id, int *params,
                 wifi_scan_result_handler handler)
-        : WifiCommand(iface, id), mParams(params), mHandler(handler)
+        : WifiCommand("FullScanResultsCommand", iface, id), mParams(params), mHandler(handler)
     { }
 
     int createRequest(WifiRequest& request, int subcmd, int enable) {
@@ -453,7 +453,7 @@ class ScanCommand : public WifiCommand
 public:
     ScanCommand(wifi_interface_handle iface, int id, wifi_scan_cmd_params *params,
                 wifi_scan_result_handler handler)
-        : WifiCommand(iface, id), mParams(params), mHandler(handler),
+        : WifiCommand("ScanCommand", iface, id), mParams(params), mHandler(handler),
           mLocalFullScanBuckets(0)
     { }
 
@@ -758,7 +758,7 @@ wifi_error wifi_stop_gscan(wifi_request_id id, wifi_interface_handle iface)
     wifi_handle handle = getWifiHandle(iface);
     ALOGV("Stopping GScan, wifi_request_id = %d, halHandle = %p", id, handle);
 
-    if(id == -1) {
+    if (id == -1) {
         wifi_scan_result_handler handler;
         wifi_scan_cmd_params dummy_params;
         wifi_handle handle = getWifiHandle(iface);
@@ -830,7 +830,7 @@ class GetScanResultsCommand : public WifiCommand {
 public:
     GetScanResultsCommand(wifi_interface_handle iface, byte flush,
             wifi_cached_scan_results *results, int max, int *num)
-        : WifiCommand(iface, -1), mScans(results), mMax(max), mNum(num),
+        : WifiCommand("GetScanResultsCommand", iface, -1), mScans(results), mMax(max), mNum(num),
                 mRetrieved(0), mFlush(flush), mCompleted(0), mNextScanResult(0)
     { }
 
@@ -996,7 +996,7 @@ private:
 public:
     BssidHotlistCommand(wifi_interface_handle handle, int id,
             wifi_bssid_hotlist_params params, wifi_hotlist_ap_found_handler handler)
-        : WifiCommand(handle, id), mParams(params), mHandler(handler)
+        : WifiCommand("BssidHotlistCommand", handle, id), mParams(params), mHandler(handler)
     { }
 
     int createSetupRequest(WifiRequest& request) {
@@ -1164,7 +1164,7 @@ private:
 public:
     ePNOCommand(wifi_interface_handle handle, int id,
             int num_networks, wifi_epno_network *networks, wifi_epno_handler handler)
-        : WifiCommand(handle, id), mHandler(handler)
+        : WifiCommand("ePNOCommand", handle, id), mHandler(handler)
     {
         ssid_list = networks;
         num_ssid = num_networks;
@@ -1359,7 +1359,8 @@ private:
 public:
     SignificantWifiChangeCommand(wifi_interface_handle handle, int id,
             wifi_significant_change_params params, wifi_significant_change_handler handler)
-        : WifiCommand(handle, id), mParams(params), mHandler(handler)
+        : WifiCommand("SignificantWifiChangeCommand", handle, id), mParams(params),
+            mHandler(handler)
     { }
 
     int createSetupRequest(WifiRequest& request) {
@@ -1582,7 +1583,7 @@ private:
 public:
     SSIDWhitelistCommand(wifi_interface_handle handle, int id,
             int num_networks, wifi_ssid *ssids)
-        : WifiCommand(handle, id), mNumNetworks(num_networks), mSSIDs(ssids)
+        : WifiCommand("SSIDWhitelistCommand", handle, id), mNumNetworks(num_networks), mSSIDs(ssids)
     { }
 
     int createRequest(WifiRequest& request) {
@@ -1671,7 +1672,7 @@ private:
     wifi_roam_params *mParams;
 public:
     RoamParamsCommand(wifi_interface_handle handle, int id, wifi_roam_params *params)
-        : WifiCommand(handle, id), mParams(params)
+        : WifiCommand("RoamParamsCommand", handle, id), mParams(params)
     { }
 
     int createRequest(WifiRequest& request) {
@@ -1763,7 +1764,7 @@ private:
     int mEnable;
 public:
     LazyRoamCommand(wifi_interface_handle handle, int id, int enable)
-        : WifiCommand(handle, id), mEnable(enable)
+        : WifiCommand("LazyRoamCommand", handle, id), mEnable(enable)
     { }
 
     int createRequest(WifiRequest& request) {
@@ -1829,7 +1830,7 @@ private:
 public:
     BssidBlacklistCommand(wifi_interface_handle handle, int id,
             wifi_bssid_params *params)
-        : WifiCommand(handle, id), mParams(params)
+        : WifiCommand("BssidBlacklistCommand", handle, id), mParams(params)
     { }
      int createRequest(WifiRequest& request) {
         int result = request.create(GOOGLE_OUI, WIFI_SUBCMD_SET_BSSID_BLACKLIST);
@@ -1908,7 +1909,7 @@ private:
 public:
     BssidPreferenceCommand(wifi_interface_handle handle, int id,
             int num_bssid, wifi_bssid_preference *prefs)
-        : WifiCommand(handle, id), mNumBssid(num_bssid), mPrefs(prefs)
+        : WifiCommand("BssidPreferenceCommand", handle, id), mNumBssid(num_bssid), mPrefs(prefs)
     { }
 
     int createRequest(WifiRequest& request) {
@@ -2008,7 +2009,8 @@ class AnqpoConfigureCommand : public WifiCommand
 public:
     AnqpoConfigureCommand(wifi_request_id id, wifi_interface_handle iface,
         int num, wifi_passpoint_network *hs_list, wifi_passpoint_event_handler handler)
-        : WifiCommand(iface, id), num_hs(num), mNetworks(hs_list), mHandler(handler)
+        : WifiCommand("AnqpoConfigureCommand", iface, id), num_hs(num), mNetworks(hs_list),
+            mHandler(handler)
     {
     }
 
diff --git a/bcmdhd/wifi_hal/link_layer_stats.cpp b/bcmdhd/wifi_hal/link_layer_stats.cpp
index 25502af..170c791 100644
--- a/bcmdhd/wifi_hal/link_layer_stats.cpp
+++ b/bcmdhd/wifi_hal/link_layer_stats.cpp
@@ -36,7 +36,7 @@ class GetLinkStatsCommand : public WifiCommand
     wifi_stats_result_handler mHandler;
 public:
     GetLinkStatsCommand(wifi_interface_handle iface, wifi_stats_result_handler handler)
-        : WifiCommand(iface, 0), mHandler(handler)
+        : WifiCommand("GetLinkStatsCommand", iface, 0), mHandler(handler)
     { }
 
     virtual int create() {
diff --git a/bcmdhd/wifi_hal/rtt.cpp b/bcmdhd/wifi_hal/rtt.cpp
index 9812fe5..f77fdb0 100644
--- a/bcmdhd/wifi_hal/rtt.cpp
+++ b/bcmdhd/wifi_hal/rtt.cpp
@@ -115,7 +115,7 @@ class GetRttCapabilitiesCommand : public WifiCommand
     wifi_rtt_capabilities *mCapabilities;
 public:
     GetRttCapabilitiesCommand(wifi_interface_handle iface, wifi_rtt_capabilities *capabitlites)
-        : WifiCommand(iface, 0), mCapabilities(capabitlites)
+        : WifiCommand("GetRttCapabilitiesCommand", iface, 0), mCapabilities(capabitlites)
     {
         memset(mCapabilities, 0, sizeof(*mCapabilities));
     }
@@ -170,7 +170,7 @@ class RttCommand : public WifiCommand
 public:
     RttCommand(wifi_interface_handle iface, int id, unsigned num_rtt_config,
             wifi_rtt_config rtt_config[], wifi_rtt_event_handler handler)
-        : WifiCommand(iface, id), numRttParams(num_rtt_config), rttParams(rtt_config),
+        : WifiCommand("RttCommand", iface, id), numRttParams(num_rtt_config), rttParams(rtt_config),
         rttHandler(handler)
     {
         memset(rttResults, 0, sizeof(rttResults));
@@ -180,7 +180,7 @@ public:
     }
 
     RttCommand(wifi_interface_handle iface, int id)
-        : WifiCommand(iface, id)
+        : WifiCommand("RttCommand", iface, id)
     {
         currentIdx = 0;
         mCompleted = 0;
diff --git a/bcmdhd/wifi_hal/wifi_hal.cpp b/bcmdhd/wifi_hal/wifi_hal.cpp
index 1d02b7b..8caa1d6 100644
--- a/bcmdhd/wifi_hal/wifi_hal.cpp
+++ b/bcmdhd/wifi_hal/wifi_hal.cpp
@@ -292,18 +292,32 @@ void wifi_cleanup(wifi_handle handle, wifi_cleaned_up_handler handler)
 
     pthread_mutex_lock(&info->cb_lock);
 
-    for (int i = 0; i < info->num_event_cb; i++) {
-        cb_info *cbi = &(info->event_cb[i]);
-        WifiCommand *cmd = (WifiCommand *)cbi->cb_arg;
+    int bad_commands = 0;
+
+    while (info->num_cmd > bad_commands) {
+        int num_cmd = info->num_cmd;
+        cmd_info *cmdi = &(info->cmd[bad_commands]);
+        WifiCommand *cmd = cmdi->cmd;
         if (cmd != NULL) {
-            cmd->addRef();
+            ALOGE("Cancelling command %p:%s", cmd, cmd->getType());
             pthread_mutex_unlock(&info->cb_lock);
             cmd->cancel();
             pthread_mutex_lock(&info->cb_lock);
+            /* release reference added when command is saved */
             cmd->releaseRef();
+            if (num_cmd == info->num_cmd) {
+                ALOGE("Cancelling command %p:%s did not work", cmd, cmd->getType());
+                bad_commands++;
+            }
         }
     }
 
+    for (int i = 0; i < info->num_event_cb; i++) {
+        cb_info *cbi = &(info->event_cb[i]);
+        WifiCommand *cmd = (WifiCommand *)cbi->cb_arg;
+        ALOGE("Leaked command %p:%s", cmd, cmd->getType());
+    }
+
     pthread_mutex_unlock(&info->cb_lock);
 
     if (write(info->cleanup_socks[0], "T", 1) < 1) {
@@ -459,7 +473,7 @@ private:
     int   mId;
 public:
     GetMulticastIdCommand(wifi_handle handle, const char *name, const char *group)
-        : WifiCommand(handle, 0)
+        : WifiCommand("GetMulticastIdCommand", handle, 0)
     {
         mName = name;
         mGroup = group;
@@ -534,7 +548,7 @@ private:
     int set_size_max;
 public:
     SetPnoMacAddrOuiCommand(wifi_interface_handle handle, oui scan_oui)
-        : WifiCommand(handle, 0)
+        : WifiCommand("SetPnoMacAddrOuiCommand", handle, 0)
     {
         mOui = scan_oui;
     }
@@ -586,7 +600,7 @@ private:
     u32 mNoDfs;
 public:
     SetNodfsCommand(wifi_interface_handle handle, u32 nodfs)
-        : WifiCommand(handle, 0) {
+        : WifiCommand("SetNodfsCommand", handle, 0) {
         mNoDfs = nodfs;
     }
     virtual int create() {
@@ -614,7 +628,7 @@ private:
     const char *mCountryCode;
 public:
     SetCountryCodeCommand(wifi_interface_handle handle, const char *country_code)
-        : WifiCommand(handle, 0) {
+        : WifiCommand("SetCountryCodeCommand", handle, 0) {
         mCountryCode = country_code;
         }
     virtual int create() {
@@ -646,7 +660,8 @@ private:
 public:
     SetRSSIMonitorCommand(wifi_request_id id, wifi_interface_handle handle,
                 s8 max_rssi, s8 min_rssi, wifi_rssi_event_handler eh)
-        : WifiCommand(handle, id), mMax_rssi(max_rssi), mMin_rssi(min_rssi), mHandler(eh)
+        : WifiCommand("SetRSSIMonitorCommand", handle, id), mMax_rssi(max_rssi), mMin_rssi
+        (min_rssi), mHandler(eh)
         {
         }
    int createRequest(WifiRequest& request, int enable) {
@@ -764,7 +779,7 @@ private:
 public:
     GetFeatureSetCommand(wifi_interface_handle handle, int feature, feature_set *set,
          feature_set set_matrix[], int *size, int max_size)
-        : WifiCommand(handle, 0)
+        : WifiCommand("GetFeatureSetCommand", handle, 0)
     {
         feature_type = feature;
         fset = set;
diff --git a/bcmdhd/wifi_hal/wifi_logger.cpp b/bcmdhd/wifi_hal/wifi_logger.cpp
index 1ef307c..5d4894b 100644
--- a/bcmdhd/wifi_hal/wifi_logger.cpp
+++ b/bcmdhd/wifi_hal/wifi_logger.cpp
@@ -94,33 +94,34 @@ public:
     // constructor for get version
     DebugCommand(wifi_interface_handle iface, char *buffer, int *buffer_size,
             GetCmdType cmdType)
-        : WifiCommand(iface, 0), mBuff(buffer), mBuffSize(buffer_size), mType(cmdType)
+        : WifiCommand("DebugCommand", iface, 0), mBuff(buffer), mBuffSize(buffer_size), mType
+        (cmdType)
     {
         memset(mBuff, 0, *mBuffSize);
     }
 
     // constructor for ring data
     DebugCommand(wifi_interface_handle iface, char *ring_name, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mRingName(ring_name), mType(cmdType)
+        : WifiCommand("DebugCommand", iface, 0), mRingName(ring_name), mType(cmdType)
     { }
 
     // constructor for ring status
     DebugCommand(wifi_interface_handle iface, u32 *num_rings,
             wifi_ring_buffer_status *status, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mNumRings(num_rings), mStatus(status), mType(cmdType)
+        : WifiCommand("DebugCommand", iface, 0), mNumRings(num_rings), mStatus(status), mType(cmdType)
     {
         memset(mStatus, 0, sizeof(wifi_ring_buffer_status) * (*mNumRings));
     }
 
     // constructor for feature set
     DebugCommand(wifi_interface_handle iface, unsigned int *support, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mSupport(support), mType(cmdType)
+        : WifiCommand("DebugCommand", iface, 0), mSupport(support), mType(cmdType)
     { }
 
     // constructor for ring params
     DebugCommand(wifi_interface_handle iface, u32 verbose_level, u32 flags,
             u32 max_interval_sec, u32 min_data_size, char *ring_name, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mVerboseLevel(verbose_level), mFlags(flags),
+        : WifiCommand("DebugCommand", iface, 0), mVerboseLevel(verbose_level), mFlags(flags),
         mMaxIntervalSec(max_interval_sec), mMinDataSize(min_data_size),
         mRingName(ring_name), mType(cmdType)
     { }
@@ -444,10 +445,10 @@ class SetLogHandler : public WifiCommand
 
 public:
     SetLogHandler(wifi_interface_handle iface, int id, wifi_ring_buffer_data_handler handler)
-        : WifiCommand(iface, id), mHandler(handler)
+        : WifiCommand("SetLogHandler", iface, id), mHandler(handler)
     { }
     SetLogHandler(wifi_interface_handle iface, int id)
-        : WifiCommand(iface, id)
+        : WifiCommand("SetLogHandler", iface, id)
     { }
 
     int start() {
@@ -564,7 +565,8 @@ class SetAlertHandler : public WifiCommand
 
 public:
     SetAlertHandler(wifi_interface_handle iface, int id, wifi_alert_handler handler)
-        : WifiCommand(iface, id), mHandler(handler), mBuffSize(0), mBuff(NULL), mErrCode(0)
+        : WifiCommand("SetAlertHandler", iface, id), mHandler(handler), mBuffSize(0), mBuff(NULL),
+            mErrCode(0)
     { }
 
     int start() {
@@ -706,7 +708,7 @@ class MemoryDumpCommand: public WifiCommand
 
 public:
     MemoryDumpCommand(wifi_interface_handle iface, wifi_firmware_memory_dump_handler handler)
-        : WifiCommand(iface, 0), mHandler(handler), mBuffSize(0), mBuff(NULL)
+        : WifiCommand("MemoryDumpCommand", iface, 0), mHandler(handler), mBuffSize(0), mBuff(NULL)
     { }
 
     int start() {
diff --git a/bcmdhd/wifi_hal/wifi_offload.cpp b/bcmdhd/wifi_hal/wifi_offload.cpp
index 936ca40..41cc13f 100644
--- a/bcmdhd/wifi_hal/wifi_offload.cpp
+++ b/bcmdhd/wifi_hal/wifi_offload.cpp
@@ -63,14 +63,14 @@ public:
     // constructor for start sending
     MKeepAliveCommand(wifi_interface_handle iface, u8 index, u8 *ip_packet, u16 ip_packet_len,
             u8 *src_mac_addr, u8 *dst_mac_addr, u32 period_msec, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mIndex(index), mIpPkt(ip_packet), mIpPktLen(ip_packet_len),
-        mSrcMacAddr(src_mac_addr), mDstMacAddr(dst_mac_addr), mPeriodMsec(period_msec),
-        mType(cmdType)
+        : WifiCommand("MKeepAliveCommand", iface, 0), mIndex(index), mIpPkt(ip_packet),
+        mIpPktLen(ip_packet_len), mSrcMacAddr(src_mac_addr), mDstMacAddr(dst_mac_addr),
+        mPeriodMsec(period_msec), mType(cmdType)
     { }
 
     // constructor for stop sending
     MKeepAliveCommand(wifi_interface_handle iface, u8 index, GetCmdType cmdType)
-        : WifiCommand(iface, 0), mIndex(index), mType(cmdType)
+        : WifiCommand("MKeepAliveCommand", iface, 0), mIndex(index), mType(cmdType)
     { }
 
     int createRequest(WifiRequest &request) {
-- 
cgit v1.1