From 96d4834e2dc6cf926cad639bb232b8b787cb8d3e Mon Sep 17 00:00:00 2001 From: Mandeep Shetty Date: Thu, 24 Sep 2015 13:42:39 -0700 Subject: [PATCH] Fix segfault when calling OCCancel () This fixes the client side crash in IOT-733. OCCancel () allocates a pointer that is free'd in routingutility.c. Although the pointer was passed in as reference into routingutility and is re-assigned with a new malloc'ed pointer, OCCancel() free's the old pointer. This is because there two structs that are being passed around. The pointer that is double free'd is free'd twice as part of two DIFFERENT structs. Got rid of the extra struct to keep things consistent and resolve the crash. If accepted, this changeset should be cherrypicked to 1.0.0-dev. Change-Id: I8c961776406c02dbe7706a787bb19db53ba83853 Signed-off-by: Mandeep Shetty Reviewed-on: https://gerrit.iotivity.org/gerrit/3055 Tested-by: jenkins-iotivity Reviewed-by: Jon A. Cruz Reviewed-by: Ashok Babu Channa Reviewed-by: Gabriel Schulhof Reviewed-by: Jaehong Jo Reviewed-by: Patrick Lankswert --- resource/csdk/stack/src/ocstack.c | 48 ++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index e8f89e4..f6d2e70 100644 --- a/resource/csdk/stack/src/ocstack.c +++ b/resource/csdk/stack/src/ocstack.c @@ -2534,7 +2534,6 @@ OCStackResult OCCancel(OCDoHandle handle, OCQualityOfService qos, OCHeaderOption */ OCStackResult ret = OC_STACK_OK; CAEndpoint_t endpoint = {.adapter = CA_DEFAULT_ADAPTER}; - CAInfo_t requestData = {.type = CA_MSG_CONFIRM}; CARequestInfo_t requestInfo = {.method = CA_GET}; if(!handle) @@ -2545,14 +2544,15 @@ OCStackResult OCCancel(OCDoHandle handle, OCQualityOfService qos, OCHeaderOption ClientCB *clientCB = GetClientCB(NULL, 0, handle, NULL); if (!clientCB) { - OC_LOG(ERROR, TAG, "Client callback not found. Called OCCancel twice?"); - goto Error; + OC_LOG(ERROR, TAG, "Callback not found. Called OCCancel on same resource twice?"); + return OC_STACK_ERROR; } switch (clientCB->method) { case OC_REST_OBSERVE: case OC_REST_OBSERVE_ALL: + OC_LOG_V(INFO, TAG, "Canceling observation for resource %s", clientCB->requestUri); if (qos != OC_HIGH_QOS) @@ -2560,29 +2560,34 @@ OCStackResult OCCancel(OCDoHandle handle, OCQualityOfService qos, OCHeaderOption FindAndDeleteClientCB(clientCB); break; } - else - { - OC_LOG(INFO, TAG, "Cancelling observation as CONFIRMABLE"); - } - requestData.type = qualityOfServiceToMessageType(qos); - requestData.token = clientCB->token; - requestData.tokenLength = clientCB->tokenLength; - if (CreateObserveHeaderOption (&(requestData.options), + OC_LOG(INFO, TAG, "Cancelling observation as CONFIRMABLE"); + + requestInfo.info.type = qualityOfServiceToMessageType(qos); + requestInfo.info.token = clientCB->token; + requestInfo.info.tokenLength = clientCB->tokenLength; + + if (CreateObserveHeaderOption (&(requestInfo.info.options), options, numOptions, OC_OBSERVE_DEREGISTER) != OC_STACK_OK) { return OC_STACK_ERROR; } - requestData.numOptions = numOptions + 1; - requestData.resourceUri = OICStrdup (clientCB->requestUri); - - requestInfo.method = CA_GET; - requestInfo.info = requestData; + requestInfo.info.numOptions = numOptions + 1; + requestInfo.info.resourceUri = OICStrdup (clientCB->requestUri); CopyDevAddrToEndpoint(clientCB->devAddr, &endpoint); - // send request ret = OCSendRequest(&endpoint, &requestInfo); + + if (requestInfo.info.options) + { + OICFree (requestInfo.info.options); + } + if (requestInfo.info.resourceUri) + { + OICFree (requestInfo.info.resourceUri); + } + break; #ifdef WITH_PRESENCE @@ -2596,15 +2601,6 @@ OCStackResult OCCancel(OCDoHandle handle, OCQualityOfService qos, OCHeaderOption break; } -Error: - if (requestData.numOptions > 0) - { - OICFree(requestData.options); - } - if (requestData.resourceUri) - { - OICFree (requestData.resourceUri); - } return ret; } -- 2.7.4