Fixed double free issue when destroying endpoint

- after destroying memory, set NULL value to prevent double free
- add the usage of oc_mutex_lock() when block data is updated

Change-Id: I07dbdff8288888ece8f89f7b278e979c09cf8e51
Signed-off-by: hyuna0213.jo <hyuna0213.jo@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/15447
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>
Reviewed-by: Jaehong Jo <jaehong.jo@samsung.com>
Reviewed-by: jihwan seo <jihwan.seo@samsung.com>
Reviewed-by: Ashok Babu Channa <ashok.channa@samsung.com>

diff --git a/resource/csdk/connectivity/inc/cablockwisetransfer.h b/resource/csdk/connectivity/inc/cablockwisetransfer.h
index 7aa81a0..dcf9cd3 100644 (file)
--- a/resource/csdk/connectivity/inc/cablockwisetransfer.h
+++ b/resource/csdk/connectivity/inc/cablockwisetransfer.h
@@ -494,6 +494,15 @@ uint8_t CAGetBlockOptionType(const CABlockDataID_t *blockID);
 CAData_t *CAGetDataSetFromBlockDataList(const CABlockDataID_t *blockID);
 
 /**
+ * Update the block data from block-wise transfer list.
+ * @param[in]   blockID     ID set of CABlockData.
+ * @param[in]   sendData    New block date should be sent.
+ * @return CAData structure.
+ */
+CABlockData_t *CAUpdateDataSetFromBlockDataList(const CABlockDataID_t *blockID,
+                                                const CAData_t *sendData);
+
+/**
  * Get token from block-wise transfer list.
  * @param[in]   pdu    received pdu binary data.
  * @param[in]   endpoint    remote endpoint information.

diff --git a/resource/csdk/connectivity/src/cablockwisetransfer.c b/resource/csdk/connectivity/src/cablockwisetransfer.c
index e206db6..bbb5be4 100644 (file)
--- a/resource/csdk/connectivity/src/cablockwisetransfer.c
+++ b/resource/csdk/connectivity/src/cablockwisetransfer.c
@@ -2199,7 +2199,7 @@ CAPayload_t CAGetPayloadInfo(const CAData_t *data, size_t *payloadLen)
             return data->requestInfo->info.payload;
         }
     }
-    else
+    else if (data->responseInfo)
     {
         if (data->responseInfo->info.payload)
         {
@@ -2309,6 +2309,31 @@ CAData_t *CAGetDataSetFromBlockDataList(const CABlockDataID_t *blockID)
     return NULL;
 }
 
+CABlockData_t *CAUpdateDataSetFromBlockDataList(const CABlockDataID_t *blockID,
+                                                const CAData_t *sendData)
+{
+    VERIFY_NON_NULL_RET(blockID, TAG, "blockID", NULL);
+    VERIFY_NON_NULL_RET(sendData, TAG, "sendData", NULL);
+
+    oc_mutex_lock(g_context.blockDataListMutex);
+
+    size_t len = u_arraylist_length(g_context.dataList);
+    for (size_t i = 0; i < len; i++)
+    {
+        CABlockData_t *currData = (CABlockData_t *) u_arraylist_get(g_context.dataList, i);
+        if (CABlockidMatches(currData, blockID))
+        {
+            CADestroyDataSet(currData->sentData);
+            currData->sentData = CACloneCAData(sendData);
+            oc_mutex_unlock(g_context.blockDataListMutex);
+            return currData;
+        }
+    }
+    oc_mutex_unlock(g_context.blockDataListMutex);
+
+    return NULL;
+}
+
 CAResult_t CAGetTokenFromBlockDataList(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint,
                                        CAResponseInfo_t *responseInfo)
 {
@@ -2379,17 +2404,11 @@ CAResult_t CACheckBlockDataValidation(const CAData_t *sendData, CABlockData_t **
             return CA_STATUS_FAILED;
         }
 
-        CABlockData_t *storedData = CAGetBlockDataFromBlockDataList(blockDataID);
-        if (storedData)
+        CABlockData_t *updatedData = CAUpdateDataSetFromBlockDataList(blockDataID, sendData);
+        if (updatedData)
         {
             OIC_LOG(DEBUG, TAG, "Send response about the received block request.");
-            if (storedData->sentData)
-            {
-                OIC_LOG(DEBUG, TAG, "init block number");
-                CADestroyDataSet(storedData->sentData);
-            }
-            storedData->sentData = CACloneCAData(sendData);
-            *blockData = storedData;
+            *blockData = updatedData;
             CADestroyBlockID(blockDataID);
             return CA_STATUS_OK;
         }
@@ -2576,10 +2595,10 @@ CAResult_t CARemoveBlockDataFromList(const CABlockDataID_t *blockID)
             }
 
             // destroy memory
-            CADestroyDataSet(currData->sentData);
-            CADestroyBlockID(currData->blockDataId);
-            OICFree(currData->payload);
-            OICFree(currData);
+            CADestroyDataSet(removedData->sentData);
+            CADestroyBlockID(removedData->blockDataId);
+            OICFree(removedData->payload);
+            OICFree(removedData);
             oc_mutex_unlock(g_context.blockDataListMutex);
             return CA_STATUS_OK;
         }
@@ -2620,14 +2639,20 @@ void CADestroyDataSet(CAData_t* data)
 {
     VERIFY_NON_NULL_VOID(data, TAG, "data");
 
-    CAFreeEndpoint(data->remoteEndpoint);
+    if (data->remoteEndpoint)
+    {
+        CAFreeEndpoint(data->remoteEndpoint);
+        data->remoteEndpoint = NULL;
+    }
     if (data->requestInfo)
     {
         CADestroyRequestInfoInternal(data->requestInfo);
+        data->requestInfo = NULL;
     }
     if (data->responseInfo)
     {
         CADestroyResponseInfoInternal(data->responseInfo);
+        data->responseInfo = NULL;
     }
     OICFree(data);
 }
@@ -2717,18 +2742,12 @@ CAResult_t CARemoveBlockDataFromListWithSeed(const CAToken_t token, uint8_t toke
         return CA_STATUS_FAILED;
     }
 
-    CAResult_t res = CA_STATUS_OK;
-
-    if (NULL != CAGetBlockDataFromBlockDataList(blockDataID))
+    CAResult_t res = CARemoveBlockDataFromList(blockDataID);
+    if (CA_STATUS_OK != res)
     {
-        res = CARemoveBlockDataFromList(blockDataID);
-        if (CA_STATUS_OK != res)
-        {
-            OIC_LOG(ERROR, TAG, "CARemoveBlockDataFromList failed");
-        }
+        OIC_LOG(ERROR, TAG, "CARemoveBlockDataFromList failed");
     }
 
     CADestroyBlockID(blockDataID);
-
     return res;
 }