Fix for crash in BLE receive thread. 24/204724/1
authorHarish Kumara M <h.marappa@samsung.com>
Fri, 23 Nov 2018 15:34:29 +0000 (21:04 +0530)
committerDoHyun Pyun <dh79.pyun@samsung.com>
Fri, 26 Apr 2019 04:12:57 +0000 (13:12 +0900)
Synchronizing access to client and server senderinfo as there are
chances of data receive handler may be acting on deleted SenderInfo
data when connection is closed. When connection closed, all senderInfo
for that connection are cleared in another thread.

https://github.sec.samsung.net/RS7-IOTIVITY/IoTivity/commit/69383ab5a059d7dd114c69d5d5ff5371b5946c98
(cherry picked from 69383ab5a059d7dd114c69d5d5ff5371b5946c98)

Change-Id: Ibc495dba0143eef908eaa35a0173dde77288d4be
Signed-off-by: Harish Kumara M <h.marappa@samsung.com>
Signed-off-by: DoHyun Pyun <dh79.pyun@samsung.com>
resource/csdk/connectivity/src/bt_le_adapter/caleadapter.c

index 01c80b2..d9ac819 100644 (file)
@@ -59,6 +59,7 @@ typedef struct
     uint32_t totalDataLen;
     uint8_t *defragData;
     CAEndpoint_t *remoteEndpoint;
+    uint8_t refCount;
 } CABLESenderInfo_t;
 
 typedef enum
@@ -74,6 +75,7 @@ typedef enum
  * default value is 20 byte.
  */
 static uint16_t g_mtuSize = CA_DEFAULT_BLE_MTU_SIZE;
+
 /**
  * Callback to provide the status of the network change to CA layer.
  */
@@ -131,7 +133,6 @@ static oc_mutex g_bleClientReceiveDataMutex = NULL;
  */
 static oc_mutex g_bleServerReceiveDataMutex = NULL;
 
-
 /**
  * Callback to be called when network packet received from either
  * GattServer or GattClient.
@@ -281,6 +282,11 @@ static u_arraylist_t *g_bleServerSenderInfo = NULL;
 static u_arraylist_t *g_bleClientSenderInfo = NULL;
 
 /**
+ * Mutex to synchronize access to all senderInfos
+ */
+static oc_mutex g_senderInfoMutex = NULL;
+
+/**
  * Queue to process the outgoing packets from GATTServer.
  */
 static CAQueueingThread_t *g_bleServerSendQueueHandle = NULL;
@@ -511,6 +517,8 @@ static void CALERemoveReceiveQueueData(u_arraylist_t *dataInfoList,
 /**
  * get received data info and positioned index from the received data list
  * for client / server which is matched same leAddress and port.
+ * CABLESenderInfo_t is reference counted structure, you must call
+ * CALEFreeSenderInfo on senderInfo to release reference.
  *
  * @param[in]  leAddress       target address to get serderInfo.
  * @param[in]  port            target port to get senderInfo.
@@ -527,6 +535,36 @@ static CAResult_t CALEGetSenderInfo(const char *leAddress,
                                     uint32_t *senderIndex);
 
 /**
+ * Add sender info to list.
+ *
+ * @param[in] u_arraylist_t   Array list to add sender info to.
+ * @param[in] senderInfo      Sender info to be added to list.
+ *
+ * @return true on success, otherwise false.
+ */
+static bool CALEAddSenderInfoToList(u_arraylist_t *senderInfoList,
+                                          CABLESenderInfo_t *senderInfo);
+
+/**
+ * Remove desired sender info from list.
+ *
+ * @param[in] u_arraylist_t   Array list to remove sender info from.
+ * @param[in] senderInfo      Sender info to be removed from list.
+ *
+ */
+static void CALERemoveSenderInfoFromList(u_arraylist_t *senderInfoList,
+                                         CABLESenderInfo_t *senderInfo);
+
+/**
+ * Free sender info. CABLESenderInfo_t is reference counted structure
+ * and info will be freed only when reference count reaches 0 or negative.
+ *
+ * @param[in] senderInfo      Sender info to be freed.
+ *
+ */
+static CAResult_t CALEFreeSenderInfo(CABLESenderInfo_t *senderInfo);
+
+/**
  * get ports related to remote address. It is need because multi application
  * can have more than 2 senderInfo using same BLE address. So before remove
  * receive queue data, should get port list from sender Info.
@@ -542,7 +580,6 @@ static CAResult_t CALEGetPortsFromSenderInfo(const char *leAddress,
 
 static CAResult_t CAInitLEServerQueues()
 {
-    
     CAResult_t result = CAInitLEServerSenderQueue();
     if (CA_STATUS_OK != result)
     {
@@ -713,6 +750,7 @@ static CAResult_t CAInitLEServerSenderQueue()
 
 static void CALEClearSenderInfoImpl(u_arraylist_t **list)
 {
+    oc_mutex_lock(g_senderInfoMutex);
     const size_t length = u_arraylist_length(*list);
     for (size_t i = 0; i < length; ++i)
     {
@@ -726,6 +764,7 @@ static void CALEClearSenderInfoImpl(u_arraylist_t **list)
          }
     }
     u_arraylist_free(list);
+    oc_mutex_unlock(g_senderInfoMutex);
 }
 
 static void CALEClearSenderInfo()
@@ -821,6 +860,7 @@ static CAResult_t CALEGetSenderInfo(const char *leAddress,
                         "NULL index argument",
                         CA_STATUS_INVALID_PARAM);
 
+    oc_mutex_lock(g_senderInfoMutex);
     const uint32_t listLength = u_arraylist_length(senderInfoList);
     const uint32_t addrLength = strlen(leAddress);
     for (uint32_t index = 0; index < listLength; index++)
@@ -831,23 +871,73 @@ static CAResult_t CALEGetSenderInfo(const char *leAddress,
             continue;
         }
 
-        if (!strncmp(info->remoteEndpoint->addr, leAddress, addrLength))
+        if (!strncmp(info->remoteEndpoint->addr, leAddress, addrLength)
+            && info->remoteEndpoint->port == port)
         {
-            if (info->remoteEndpoint->port == port)
+            *senderIndex = index;
+            if (senderInfo)
             {
-                *senderIndex = index;
-                if (senderInfo)
-                {
-                    *senderInfo = info;
-                }
-                return CA_STATUS_OK;
+                *senderInfo = info;
+                (*senderInfo)->refCount++;
             }
+            oc_mutex_unlock(g_senderInfoMutex);
+            return CA_STATUS_OK;
         }
     }
 
+    oc_mutex_unlock(g_senderInfoMutex);
     return CA_STATUS_FAILED;
 }
 
+static bool CALEAddSenderInfoToList(u_arraylist_t *senderInfoList, CABLESenderInfo_t *senderInfo)
+{
+    oc_mutex_lock(g_senderInfoMutex);
+    senderInfo->refCount++;
+    if (!u_arraylist_add(senderInfoList,(void *)senderInfo))
+    {
+        senderInfo->refCount--;
+        oc_mutex_unlock(g_senderInfoMutex);
+        return false;
+    }
+    oc_mutex_unlock(g_senderInfoMutex);
+    return true;
+}
+
+static void CALERemoveSenderInfoFromList(u_arraylist_t *senderInfoList, CABLESenderInfo_t *senderInfo)
+{
+    oc_mutex_lock(g_senderInfoMutex);
+    uint32_t idx;
+    bool elemFound = u_arraylist_get_index(senderInfoList, (const void*)senderInfo, &idx);
+    if(elemFound)
+    {
+        void *info = u_arraylist_remove(senderInfoList, idx);
+        if (info != NULL)
+        {
+            senderInfo->refCount--;
+        }
+    }
+    oc_mutex_unlock(g_senderInfoMutex);
+}
+
+static CAResult_t CALEFreeSenderInfo(CABLESenderInfo_t *senderInfo)
+{
+    VERIFY_NON_NULL_RET(senderInfo,
+                        CALEADAPTER_TAG,
+                        "NULL senderInfo to remove argument",
+                        CA_STATUS_INVALID_PARAM);
+
+    oc_mutex_lock(g_senderInfoMutex);
+    senderInfo->refCount--;
+    if(senderInfo->refCount <= 0)
+    {
+        OICFree(senderInfo->defragData);
+        OICFree(senderInfo->remoteEndpoint);
+        OICFree(senderInfo);
+    }
+    oc_mutex_unlock(g_senderInfoMutex);
+    return CA_STATUS_OK;
+}
+
 static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverType)
 {
     OIC_LOG_V(DEBUG, CALEADAPTER_TAG, "%s", __func__);
@@ -935,9 +1025,8 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG,
                         "This packet is start packet but exist senderInfo. Remove senderInfo");
-                u_arraylist_remove(bleData->senderInfo, senderIndex);
-                OICFree(senderInfo->defragData);
-                OICFree(senderInfo);
+                CALERemoveSenderInfoFromList(bleData->senderInfo, senderInfo);
+                CALEFreeSenderInfo(senderInfo);
                 senderInfo = NULL;
                 senderIndex = 0;
             }
@@ -957,26 +1046,21 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
                 return;
             }
 
-            CABLESenderInfo_t *newSender = OICMalloc(sizeof(CABLESenderInfo_t));
+            CABLESenderInfo_t *newSender = OICCalloc(1, sizeof(CABLESenderInfo_t));
             if (!newSender)
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "Memory allocation failed for new sender");
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
-            newSender->recvDataLen = 0;
-            newSender->totalDataLen = 0;
-            newSender->defragData = NULL;
-            newSender->remoteEndpoint = NULL;
 
             OIC_LOG(DEBUG, CALEADAPTER_TAG, "Parsing the header");
 
             newSender->totalDataLen = totalLength;
-
             if (!(newSender->totalDataLen))
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "Total Data Length is parsed as 0!!!");
-                OICFree(newSender);
+                CALEFreeSenderInfo(newSender);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -994,7 +1078,7 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             if (NULL == newSender->defragData)
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "defragData is NULL!");
-                OICFree(newSender);
+                CALEFreeSenderInfo(newSender);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -1015,8 +1099,7 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             if (NULL == newSender->remoteEndpoint)
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "remoteEndpoint is NULL!");
-                OICFree(newSender->defragData);
-                OICFree(newSender);
+                CALEFreeSenderInfo(newSender);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -1024,9 +1107,7 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             if (newSender->recvDataLen + dataOnlyLen > newSender->totalDataLen)
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "buffer is smaller than received data");
-                OICFree(newSender->defragData);
-                CAFreeEndpoint(newSender->remoteEndpoint);
-                OICFree(newSender);
+                CALEFreeSenderInfo(newSender);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -1035,22 +1116,16 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
                    dataOnlyLen);
             newSender->recvDataLen += dataOnlyLen;
 
-            u_arraylist_add(bleData->senderInfo,(void *)newSender);
+            newSender->refCount = 1;
 
-            //Getting newSender index position in bleSenderInfo array list
-            if (CA_STATUS_OK !=
-                    CALEGetSenderInfo(newSender->remoteEndpoint->addr,
-                                      newSender->remoteEndpoint->port,
-                                      bleData->senderInfo,
-                                      NULL, &senderIndex))
+            if (!CALEAddSenderInfoToList(bleData->senderInfo, newSender))
             {
-                OIC_LOG(ERROR, CALEADAPTER_TAG, "Existing sender index not found!!");
-                OICFree(newSender->defragData);
-                CAFreeEndpoint(newSender->remoteEndpoint);
-                OICFree(newSender);
+                OIC_LOG(ERROR, CALEADAPTER_TAG, "Failed to add sender info!");
+                CALEFreeSenderInfo(newSender);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
+
             senderInfo = newSender;
         }
         else
@@ -1061,9 +1136,8 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
                 OIC_LOG_V(ERROR, CALEADAPTER_TAG,
                           "Data Length exceeding error!! Receiving [%zu] total length [%u]",
                           senderInfo->recvDataLen + dataOnlyLen, senderInfo->totalDataLen);
-                u_arraylist_remove(bleData->senderInfo, senderIndex);
-                OICFree(senderInfo->defragData);
-                OICFree(senderInfo);
+                CALERemoveSenderInfoFromList(bleData->senderInfo, senderInfo);
+                CALEFreeSenderInfo(senderInfo);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -1082,10 +1156,8 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             if (NULL == g_networkPacketReceivedCallback)
             {
                 OIC_LOG(ERROR, CALEADAPTER_TAG, "gReqRespCallback is NULL!");
-
-                u_arraylist_remove(bleData->senderInfo, senderIndex);
-                OICFree(senderInfo->defragData);
-                OICFree(senderInfo);
+                CALERemoveSenderInfoFromList(bleData->senderInfo, senderInfo);
+                CALEFreeSenderInfo(senderInfo);
                 oc_mutex_unlock(bleReceiveDataMutex);
                 return;
             }
@@ -1112,10 +1184,10 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
                         break;
                     default:
                         OIC_LOG_V(ERROR, CALEADAPTER_TAG, "Unsupported rcvr type:%d",receiverType);
-                        u_arraylist_remove(bleData->senderInfo, senderIndex);
+                        CALERemoveSenderInfoFromList(bleData->senderInfo, senderInfo);
                         senderInfo->remoteEndpoint = NULL;
                         senderInfo->defragData = NULL;
-                        OICFree(senderInfo);
+                        CALEFreeSenderInfo(senderInfo);
                         oc_mutex_unlock(bleReceiveDataMutex);
                         return;
                 }
@@ -1143,11 +1215,14 @@ static void CALEDataReceiverHandler(void *threadData, CABLEAdapter_t receiverTyp
             }
 #endif
 
-            u_arraylist_remove(bleData->senderInfo, senderIndex);
+            CALERemoveSenderInfoFromList(bleData->senderInfo, senderInfo);
             senderInfo->remoteEndpoint = NULL;
             senderInfo->defragData = NULL;
-            OICFree(senderInfo);
+            CALEFreeSenderInfo(senderInfo);
+            oc_mutex_unlock(bleReceiveDataMutex);
+            return;
         }
+        CALEFreeSenderInfo(senderInfo);
     }
     oc_mutex_unlock(bleReceiveDataMutex);
 }
@@ -2154,6 +2229,16 @@ static CAResult_t CAInitLEAdapterMutex()
         }
     }
 
+    if (NULL == g_senderInfoMutex)
+    {
+        g_senderInfoMutex = oc_mutex_new();
+        if (NULL == g_senderInfoMutex)
+        {
+            OIC_LOG(ERROR, CALEADAPTER_TAG, "oc_mutex_new failed");
+            return CA_STATUS_FAILED;
+        }
+    }
+
     return CA_STATUS_OK;
 }
 
@@ -2168,13 +2253,14 @@ static void CATerminateLEAdapterMutex()
     oc_mutex_free(g_bleLocalAddressMutex);
     g_bleLocalAddressMutex = NULL;
 
-    
-
     oc_mutex_free(g_bleServerReceiveDataMutex);
     g_bleServerReceiveDataMutex = NULL;
 
     oc_mutex_free(g_bleClientReceiveDataMutex);
     g_bleClientReceiveDataMutex = NULL;
+
+    oc_mutex_free(g_senderInfoMutex);
+    g_senderInfoMutex = NULL;
 }
 
 /**
@@ -3618,13 +3704,10 @@ static void CALERemoveReceiveQueueData(u_arraylist_t *dataInfoList, const char*
                                                   dataInfoList, &senderInfo,
                                                   &senderIndex))
             {
-                u_arraylist_remove(dataInfoList, senderIndex);
-                OICFree(senderInfo->defragData);
-                OICFree(senderInfo->remoteEndpoint);
-                OICFree(senderInfo);
-
                 OIC_LOG(DEBUG, CALEADAPTER_TAG,
                         "SenderInfo is removed for disconnection");
+                CALERemoveSenderInfoFromList(dataInfoList, senderInfo);
+                CALEFreeSenderInfo(senderInfo);
             }
             else
             {