From 08f2d0b32224b633bc92705353e1d60663a4a681 Mon Sep 17 00:00:00 2001 From: Woochul Shim Date: Tue, 6 Oct 2015 09:41:44 +0900 Subject: [PATCH] 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/3543 Reviewed-by: Randeep Singh Tested-by: jenkins-iotivity Reviewed-by: Sachin Agrawal --- .../csdk/security/provisioning/src/pmutility.c | 34 +++++++++++++++------- resource/csdk/stack/src/ocstack.c | 14 ++++++--- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/resource/csdk/security/provisioning/src/pmutility.c b/resource/csdk/security/provisioning/src/pmutility.c index 59ae010..f6e2ff3 100644 --- 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 --- 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); -- 2.7.4