Fix segmentation fault problem after discovery.

- PMDeviceDiscovery() add discovered devices in the list
  whenever callback is hit. If timeout happens the pointer to a list
  could be freed by the app but still there is a registered discovery callback.
  If the callback is hit after defined timeout period, callback try to
  add a discovered device to the invalid pointer (to freed memory).
  It will cause segmentation fault.

Fix: After defined timeout, PMDeviceDiscovery() explicitly
     remove registered callback using OCCancel.

Change-Id: Ic713cefb75c3495cfc9ad6688df44eb124d88f82
Signed-off-by: Woochul Shim <woochul.shim@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/3545
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Sachin Agrawal <sachin.agrawal@intel.com>

diff --git a/resource/csdk/security/provisioning/src/pmutility.c b/resource/csdk/security/provisioning/src/pmutility.c
index 59ae010..f6e2ff3 100644 (file)
--- a/resource/csdk/security/provisioning/src/pmutility.c
+++ b/resource/csdk/security/provisioning/src/pmutility.c
@@ -400,7 +400,7 @@ static OCStackApplicationResult SecurePortDiscoveryHandler(void *ctx, OCDoHandle
     if (ctx == NULL)
     {
         OC_LOG(ERROR, TAG, "Lost List of device information");
-        return OC_STACK_KEEP_TRANSACTION;
+        return OC_STACK_DELETE_TRANSACTION;
     }
     (void)UNUSED;
     if (clientResponse)
@@ -414,7 +414,7 @@ static OCStackApplicationResult SecurePortDiscoveryHandler(void *ctx, OCDoHandle
             if (PAYLOAD_TYPE_DISCOVERY != clientResponse->payload->type)
             {
                 OC_LOG(INFO, TAG, "Wrong payload type");
-                return OC_STACK_KEEP_TRANSACTION;
+                return OC_STACK_DELETE_TRANSACTION;
             }
 
             uint16_t securePort = 0;
@@ -427,7 +427,7 @@ static OCStackApplicationResult SecurePortDiscoveryHandler(void *ctx, OCDoHandle
             else
             {
                 OC_LOG(INFO, TAG, "Can not find secure port information.");
-                return OC_STACK_KEEP_TRANSACTION;
+                return OC_STACK_DELETE_TRANSACTION;
             }
 
             DiscoveryInfo* pDInfo = (DiscoveryInfo*)ctx;
@@ -438,12 +438,12 @@ static OCStackApplicationResult SecurePortDiscoveryHandler(void *ctx, OCDoHandle
             if (OC_STACK_OK != res)
             {
                 OC_LOG(ERROR, TAG, "Error while getting secure port.");
-                return OC_STACK_KEEP_TRANSACTION;
+                return OC_STACK_DELETE_TRANSACTION;
             }
             OC_LOG(INFO, TAG, "Exiting SecurePortDiscoveryHandler.");
         }
 
-        return  OC_STACK_KEEP_TRANSACTION;
+        return  OC_STACK_DELETE_TRANSACTION;
     }
     else
     {
@@ -619,12 +619,14 @@ OCStackResult PMDeviceDiscovery(unsigned short waittime, bool isOwned, OCProvisi
     const char* query = isOwned ? DOXM_OWNED_TRUE_MULTICAST_QUERY :
                                   DOXM_OWNED_FALSE_MULTICAST_QUERY;
 
-    res = OCDoResource(NULL, OC_REST_DISCOVER, query, 0, 0,
+    OCDoHandle handle = NULL;
+    res = OCDoResource(&handle, OC_REST_DISCOVER, query, 0, 0,
                                      CT_DEFAULT, OC_LOW_QOS, &cbData, NULL, 0);
     if (res != OC_STACK_OK)
     {
         OC_LOG(ERROR, TAG, "OCStack resource error");
-        goto exit;
+        OICFree(pDInfo);
+        return res;
     }
 
     //Waiting for each response.
@@ -632,12 +634,22 @@ OCStackResult PMDeviceDiscovery(unsigned short waittime, bool isOwned, OCProvisi
     if(OC_STACK_OK != res)
     {
         OC_LOG(ERROR, TAG, "Failed to wait response for secure discovery.");
-        goto exit;
+        OICFree(pDInfo);
+        OCStackResult resCancel = OCCancel(handle, OC_LOW_QOS, NULL, 0);
+        if(OC_STACK_OK !=  resCancel)
+        {
+            OC_LOG(ERROR, TAG, "Failed to remove registered callback");
+        }
+        return res;
+    }
+    res = OCCancel(handle,OC_LOW_QOS,NULL,0);
+    if (OC_STACK_OK != res)
+    {
+        OC_LOG(ERROR, TAG, "Failed to remove registered callback");
+        OICFree(pDInfo);
+        return res;
     }
-
     OC_LOG(DEBUG, TAG, "OUT PMDeviceDiscovery");
-
-exit:
     OICFree(pDInfo);
     return res;
 }

diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c
index d458b22..60b036b 100755 (executable)
--- a/resource/csdk/stack/src/ocstack.c
+++ b/resource/csdk/stack/src/ocstack.c
@@ -1901,18 +1901,18 @@ OCStackResult OCInit1(OCMode mode, OCTransportFlags serverFlags, OCTransportFlag
     switch (myStackMode)
     {
         case OC_CLIENT:
-                       CARegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
+            CARegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
             result = CAResultToOCResult(CAStartDiscoveryServer());
             OC_LOG(INFO, TAG, "Client mode: CAStartDiscoveryServer");
             break;
         case OC_SERVER:
-                       SRMRegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
+            SRMRegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
             result = CAResultToOCResult(CAStartListeningServer());
             OC_LOG(INFO, TAG, "Server mode: CAStartListeningServer");
             break;
         case OC_CLIENT_SERVER:
         case OC_GATEWAY:
-                       SRMRegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
+            SRMRegisterHandler(HandleCARequests, HandleCAResponses, HandleCAErrorResponse);
             result = CAResultToOCResult(CAStartListeningServer());
             if(result == OC_STACK_OK)
             {
@@ -2000,7 +2000,7 @@ OCStackResult OCStop()
     // Remove all the client callbacks
     DeleteClientCBList();
 
-       // De-init the SRM Policy Engine
+    // De-init the SRM Policy Engine
     // TODO after BeachHead delivery: consolidate into single SRMDeInit()
     SRMDeInitPolicyEngine();
 
@@ -2638,6 +2638,12 @@ OCStackResult OCCancel(OCDoHandle handle, OCQualityOfService qos, OCHeaderOption
 
             break;
 
+        case OC_REST_DISCOVER:
+            OC_LOG_V(INFO, TAG, "Cancelling discovery callback for resource %s",
+                                           clientCB->requestUri);
+            FindAndDeleteClientCB(clientCB);
+            break;
+
 #ifdef WITH_PRESENCE
         case OC_REST_PRESENCE:
             FindAndDeleteClientCB(clientCB);