From 43674525f5635a77cdfd840769894f9cda11737b Mon Sep 17 00:00:00 2001 From: KIM JungYong Date: Tue, 16 Aug 2016 14:15:31 +0900 Subject: [PATCH] Fix memory issue on calling C APIs at Cpp layer. When the calling APIs with NSProvider parameter at Cpp layer, lead to crash by illegal access of memory. This problem is due to type-cast of NSProvider between NSProvider_internal. So, type-casting of structure is removed. Conflicts: service/notification/src/consumer/NSConsumerCommunication.c service/notification/src/consumer/NSConsumerInterface.c service/notification/src/consumer/NSConsumerInternalTaskController.c Change-Id: I32c61b56510886f9f3be341d2ba497e211f0cad6 Signed-off-by: KIM JungYong Reviewed-on: https://gerrit.iotivity.org/gerrit/10487 Tested-by: jenkins-iotivity Reviewed-by: Uze Choi Tested-by: Uze Choi --- .../notification/src/consumer/NSConsumerCommon.c | 40 ++++++++++++++++++++-- .../notification/src/consumer/NSConsumerCommon.h | 6 ++-- .../src/consumer/NSConsumerCommunication.c | 8 ++--- .../src/consumer/NSConsumerDiscovery.c | 2 +- .../src/consumer/NSConsumerInterface.c | 29 ++++++++++------ .../consumer/NSConsumerInternalTaskController.c | 17 ++++----- .../src/consumer/NSConsumerMemoryCache.c | 8 ++--- .../src/consumer/NSConsumerScheduler.c | 40 +++++++++++++++++++--- 8 files changed, 114 insertions(+), 36 deletions(-) diff --git a/service/notification/src/consumer/NSConsumerCommon.c b/service/notification/src/consumer/NSConsumerCommon.c index 63af3d3..22bd30e 100644 --- a/service/notification/src/consumer/NSConsumerCommon.c +++ b/service/notification/src/consumer/NSConsumerCommon.c @@ -134,7 +134,7 @@ void NSDiscoveredProvider(NSProvider * provider) { NS_VERIFY_NOT_NULL_V(provider); - NSProvider * retProvider = (NSProvider *)NSCopyProvider((NSProvider_internal *)provider); + NSProvider * retProvider = (NSProvider *)NSCopyProvider_internal((NSProvider_internal *)provider); NS_VERIFY_NOT_NULL_V(retProvider); NSConsumerThread * thread = NSThreadInit(NSDiscoveredProviderFunc, (void *) retProvider); @@ -428,7 +428,7 @@ void NSCopyProviderPostClean( NSOICFree(provider); } -NSProvider_internal * NSCopyProvider(NSProvider_internal * prov) +NSProvider_internal * NSCopyProvider_internal(NSProvider_internal * prov) { NS_VERIFY_NOT_NULL(prov, NULL); @@ -459,7 +459,29 @@ NSProvider_internal * NSCopyProvider(NSProvider_internal * prov) return newProv; } -void NSRemoveProvider(NSProvider_internal * prov) +NSProvider * NSCopyProvider(NSProvider_internal * prov) +{ + NS_VERIFY_NOT_NULL(prov, NULL); + + NSProvider * newProv = (NSProvider *) OICMalloc(sizeof(NSProvider)); + NS_VERIFY_NOT_NULL(newProv, NULL); + + newProv->topicLL = NULL; + + if (prov->topicLL) + { + NSTopicLL * topicList = NSCopyTopicLL(prov->topicLL); + NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(topicList, NULL, NSRemoveProvider(newProv)); + + newProv->topicLL = topicList; + } + + OICStrcpy(newProv->providerId, NS_DEVICE_ID_LENGTH, prov->providerId); + + return newProv; +} + +void NSRemoveProvider_internal(NSProvider_internal * prov) { NS_VERIFY_NOT_NULL_V(prov); @@ -475,6 +497,18 @@ void NSRemoveProvider(NSProvider_internal * prov) NSOICFree(prov); } +void NSRemoveProvider(NSProvider * prov) +{ + NS_VERIFY_NOT_NULL_V(prov); + + if (prov->topicLL) + { + NSRemoveTopicLL(prov->topicLL); + } + + NSOICFree(prov); +} + NSSyncInfo_internal * NSCopySyncInfo(NSSyncInfo_internal * syncInfo) { NS_VERIFY_NOT_NULL(syncInfo, NULL); diff --git a/service/notification/src/consumer/NSConsumerCommon.h b/service/notification/src/consumer/NSConsumerCommon.h index 86cadd1..9c02e8d 100644 --- a/service/notification/src/consumer/NSConsumerCommon.h +++ b/service/notification/src/consumer/NSConsumerCommon.h @@ -134,8 +134,10 @@ NSProviderConnectionInfo * NSCreateProviderConnections(OCDevAddr *); NSProviderConnectionInfo * NSCopyProviderConnections(NSProviderConnectionInfo *); void NSRemoveConnections(NSProviderConnectionInfo *); -NSProvider_internal * NSCopyProvider(NSProvider_internal *); -void NSRemoveProvider(NSProvider_internal *); +NSProvider_internal * NSCopyProvider_internal(NSProvider_internal *); +NSProvider * NSCopyProvider(NSProvider_internal *); +void NSRemoveProvider_internal(NSProvider_internal *); +void NSRemoveProvider(NSProvider *); NSSyncInfo_internal * NSCopySyncInfo(NSSyncInfo_internal *); void NSRemoveSyncInfo(NSSyncInfo_internal *); diff --git a/service/notification/src/consumer/NSConsumerCommunication.c b/service/notification/src/consumer/NSConsumerCommunication.c index 4bb5686..75bc415 100644 --- a/service/notification/src/consumer/NSConsumerCommunication.c +++ b/service/notification/src/consumer/NSConsumerCommunication.c @@ -610,9 +610,9 @@ OCStackApplicationResult NSIntrospectTopic( (void) handle; NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(clientResponse, OC_STACK_KEEP_TRANSACTION, - NSRemoveProvider((NSProvider_internal *) ctx)); + NSRemoveProvider_internal((NSProvider_internal *) ctx)); NS_VERIFY_STACK_SUCCESS_WITH_POST_CLEANING(NSOCResultToSuccess(clientResponse->result), - OC_STACK_KEEP_TRANSACTION, NSRemoveProvider((NSProvider_internal *) ctx)); + OC_STACK_KEEP_TRANSACTION, NSRemoveProvider_internal((NSProvider_internal *) ctx)); NS_LOG_V(DEBUG, "GET response income : %s:%d", clientResponse->devAddr.addr, clientResponse->devAddr.port); @@ -627,14 +627,14 @@ OCStackApplicationResult NSIntrospectTopic( NSTopicLL * newTopicLL = NSGetTopicLL(clientResponse); NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(newTopicLL, OC_STACK_KEEP_TRANSACTION, - NSRemoveProvider((NSProvider_internal *) ctx)); + NSRemoveProvider_internal((NSProvider_internal *) ctx)); NSProvider_internal * provider = (NSProvider_internal *) ctx; provider->topicLL = NSCopyTopicLL(newTopicLL); NS_LOG(DEBUG, "build NSTask"); NSTask * task = NSMakeTask(TASK_CONSUMER_RECV_TOPIC_LIST, (void *) provider); - NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(task, NS_ERROR, NSRemoveProvider(provider)); + NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(task, NS_ERROR, NSRemoveProvider_internal(provider)); NSConsumerPushEvent(task); NSRemoveTopicLL(newTopicLL); diff --git a/service/notification/src/consumer/NSConsumerDiscovery.c b/service/notification/src/consumer/NSConsumerDiscovery.c index 6d321c9..b2f110f 100644 --- a/service/notification/src/consumer/NSConsumerDiscovery.c +++ b/service/notification/src/consumer/NSConsumerDiscovery.c @@ -158,7 +158,7 @@ OCStackApplicationResult NSIntrospectProvider( NS_LOG(DEBUG, "build NSTask"); NSTask * task = NSMakeTask(TASK_CONSUMER_PROVIDER_DISCOVERED, (void *) newProvider); - NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(task, NS_ERROR, NSRemoveProvider(newProvider)); + NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(task, NS_ERROR, NSRemoveProvider_internal(newProvider)); NSConsumerPushEvent(task); diff --git a/service/notification/src/consumer/NSConsumerInterface.c b/service/notification/src/consumer/NSConsumerInterface.c index 9b7e379..fa6cb4d 100644 --- a/service/notification/src/consumer/NSConsumerInterface.c +++ b/service/notification/src/consumer/NSConsumerInterface.c @@ -139,7 +139,14 @@ NSProvider * NSConsumerGetProvider(const char * providerId) NS_VERIFY_NOT_NULL(providerId, NULL); - return (NSProvider *) NSConsumerFindNSProvider(providerId); + NSProvider_internal * prov = NSConsumerFindNSProvider(providerId); + NS_VERIFY_NOT_NULL(prov, NULL); + + NSProvider * retProv = NSCopyProvider(prov); + NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(retProv, NULL, NSRemoveProvider_internal(prov)); + NSRemoveProvider_internal(prov); + + return retProv; } NSMessage * NSConsumerGetMessage(uint64_t messageId) @@ -160,7 +167,11 @@ NSResult NSConsumerGetInterestTopics(NSProvider * provider) NS_VERIFY_NOT_NULL(provider, NS_ERROR); - NSSelector selector = (NSSelector)((NSProvider_internal *) provider)->accessPolicy; + NSProvider_internal * prov = NSConsumerFindNSProvider(provider->providerId); + NS_VERIFY_NOT_NULL(prov, NS_ERROR); + NSSelector selector = prov->accessPolicy; + NSRemoveProvider_internal(prov); + NS_VERIFY_NOT_NULL(selector == NS_SELECTION_CONSUMER ? (void *) 1 : NULL, NS_ERROR); NSTask * topicTask = NSMakeTask(TASK_CONSUMER_GET_TOPIC_LIST, (void *) provider); @@ -175,16 +186,14 @@ NSResult NSConsumerSelectInterestTopics(NSProvider * provider) NS_VERIFY_NOT_NULL(isStartedConsumer == true ? (void *) 1 : NULL, NS_ERROR); NS_VERIFY_NOT_NULL(provider, NS_ERROR); - - NSSelector selector = (NSSelector)((NSProvider_internal *) provider)->accessPolicy; - NS_VERIFY_NOT_NULL(selector == NS_SELECTION_CONSUMER ? (void *) 1 : NULL, NS_ERROR); - - if (!provider->topicLL) - { - provider->topicLL = (NSTopicLL *) OICMalloc(sizeof(NSTopicLL)); - } NS_VERIFY_NOT_NULL(provider->topicLL, NS_ERROR); + NSProvider_internal * prov = NSConsumerFindNSProvider(provider->providerId); + NS_VERIFY_NOT_NULL(prov, NS_ERROR); + + NSSelector selector = prov->accessPolicy; + NSRemoveProvider_internal(prov); + NS_VERIFY_NOT_NULL(selector == NS_SELECTION_CONSUMER ? (void *) 1 : NULL, NS_ERROR); NSTask * topicTask = NSMakeTask(TASK_CONSUMER_SELECT_TOPIC_LIST, (void *) provider); NS_VERIFY_NOT_NULL(provider, NS_ERROR); diff --git a/service/notification/src/consumer/NSConsumerInternalTaskController.c b/service/notification/src/consumer/NSConsumerInternalTaskController.c index 3973b59..e6f285d 100644 --- a/service/notification/src/consumer/NSConsumerInternalTaskController.c +++ b/service/notification/src/consumer/NSConsumerInternalTaskController.c @@ -109,7 +109,7 @@ NSProvider_internal * NSProviderCacheFind(const char * providerId) NSCacheElement * cacheElement = NSStorageRead(ProviderCache, providerId); NS_VERIFY_NOT_NULL(cacheElement, NULL); - return NSCopyProvider((NSProvider_internal *) cacheElement->data); + return NSCopyProvider_internal((NSProvider_internal *) cacheElement->data); } void NSRemoveCacheElementMessage(NSCacheElement * obj) @@ -257,19 +257,20 @@ void NSConsumerHandleProviderDiscovered(NSProvider_internal * provider) if (provider->accessPolicy == NS_SELECTION_CONSUMER && isSubscribing == false) { NS_LOG(DEBUG, "accepter is NS_ACCEPTER_CONSUMER, Callback to user"); - NSDiscoveredProvider((NSProvider *) provider); + NSProvider * providerForCb = NSCopyProvider(provider); + NSDiscoveredProvider(providerForCb); } else { NS_LOG(DEBUG, "accepter is NS_ACCEPTER_PROVIDER, request subscribe"); - NSProvider_internal * subProvider = NSCopyProvider(provider); + NSProvider_internal * subProvider = NSCopyProvider_internal(provider); NSTask * task = NSMakeTask(TASK_CONSUMER_REQ_SUBSCRIBE, (void *) subProvider); NS_VERIFY_NOT_NULL_V(task); NSConsumerPushEvent(task); } - NSRemoveProvider(providerCacheData); + NSRemoveProvider_internal(providerCacheData); } void NSConsumerHandleProviderDeleted(NSProvider_internal * provider) @@ -357,7 +358,7 @@ void NSConsumerHandleGetTopicUri(NSMessage * msg) NS_VERIFY_NOT_NULL_V(provider); NSTask * topicTask = NSMakeTask(TASK_CONSUMER_REQ_TOPIC_LIST, (void *) provider); - NS_VERIFY_NOT_NULL_WITH_POST_CLEANING_V(topicTask, NSRemoveProvider(provider)); + NS_VERIFY_NOT_NULL_WITH_POST_CLEANING_V(topicTask, NSRemoveProvider_internal(provider)); NSConsumerPushEvent(topicTask); } @@ -403,7 +404,7 @@ void NSConsumerInternalTaskProcessing(NSTask * task) { NS_LOG(DEBUG, "Receive New Provider is discovered."); NSConsumerHandleProviderDiscovered((NSProvider_internal *)task->taskData); - NSRemoveProvider((NSProvider_internal *)task->taskData); + NSRemoveProvider_internal((NSProvider_internal *)task->taskData); break; } case TASK_RECV_SYNCINFO: @@ -431,14 +432,14 @@ void NSConsumerInternalTaskProcessing(NSTask * task) { NS_LOG(DEBUG, "Receive Topic List"); NSConsumerHandleRecvTopicLL((NSProvider_internal *)task->taskData); - NSRemoveProvider((NSProvider_internal *)task->taskData); + NSRemoveProvider_internal((NSProvider_internal *)task->taskData); break; } case TASK_CONSUMER_REQ_SUBSCRIBE_CANCEL: { NS_LOG(DEBUG, "Make Subscribe cancel from provider."); NSConsumerHandleProviderDeleted((NSProvider_internal *)task->taskData); - NSRemoveProvider((NSProvider_internal *)task->taskData); + NSRemoveProvider_internal((NSProvider_internal *)task->taskData); break; } default : diff --git a/service/notification/src/consumer/NSConsumerMemoryCache.c b/service/notification/src/consumer/NSConsumerMemoryCache.c index a9e0c27..38fce40 100644 --- a/service/notification/src/consumer/NSConsumerMemoryCache.c +++ b/service/notification/src/consumer/NSConsumerMemoryCache.c @@ -131,7 +131,7 @@ NSResult NSStorageDelete(NSCacheList * list, const char * delId) } else if (type == NS_CONSUMER_CACHE_PROVIDER) { - NSRemoveProvider((NSProvider_internal *) del->data); + NSRemoveProvider_internal((NSProvider_internal *) del->data); } NSOICFree(del); pthread_mutex_unlock(mutex); @@ -157,7 +157,7 @@ NSResult NSStorageDelete(NSCacheList * list, const char * delId) } else if (type == NS_CONSUMER_CACHE_PROVIDER) { - NSRemoveProvider((NSProvider_internal *) del->data); + NSRemoveProvider_internal((NSProvider_internal *) del->data); } NSOICFree(del); pthread_mutex_unlock(mutex); @@ -278,7 +278,7 @@ NSResult NSConsumerCacheWriteProvider(NSCacheList * list, NSCacheElement * newOb NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(obj, NS_ERROR, pthread_mutex_unlock(mutex)); NS_LOG_V(DEBUG, "New Object address : %s:%d", newProvObj->connection->addr->addr, newProvObj->connection->addr->port); - obj->data = (void *) NSCopyProvider(newProvObj); + obj->data = (void *) NSCopyProvider_internal(newProvObj); NS_LOG (DEBUG, "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!2"); prov = (NSProvider_internal *)obj->data; @@ -379,7 +379,7 @@ NSResult NSStorageDestroy(NSCacheList * list) { next = (NSCacheElement *) iter->next; - NSRemoveProvider((NSProvider_internal *) iter->data); + NSRemoveProvider_internal((NSProvider_internal *) iter->data); NSOICFree(iter); iter = next; diff --git a/service/notification/src/consumer/NSConsumerScheduler.c b/service/notification/src/consumer/NSConsumerScheduler.c index 6f4b92d..1e4b5f8 100644 --- a/service/notification/src/consumer/NSConsumerScheduler.c +++ b/service/notification/src/consumer/NSConsumerScheduler.c @@ -223,22 +223,54 @@ void NSConsumerTaskProcessing(NSTask * task) break; } case TASK_CONSUMER_REQ_SUBSCRIBE: + { + NSProvider_internal * prov = + NSConsumerFindNSProvider(((NSProvider *)task->taskData)->providerId); + NS_VERIFY_NOT_NULL_V(prov); + NSTask * subTask = NSMakeTask(TASK_CONSUMER_REQ_SUBSCRIBE, prov); + NS_VERIFY_NOT_NULL_V(subTask); + NSConsumerCommunicationTaskProcessing(subTask); + + NSRemoveProvider((NSProvider *)task->taskData); + NSOICFree(task); + break; + } case TASK_SEND_SYNCINFO: case TASK_CONSUMER_REQ_TOPIC_LIST: + { + NSConsumerCommunicationTaskProcessing(task); + break; + } case TASK_CONSUMER_GET_TOPIC_LIST: case TASK_CONSUMER_SELECT_TOPIC_LIST: { - NSConsumerCommunicationTaskProcessing(task); + NSProvider_internal * prov = + NSConsumerFindNSProvider(((NSProvider *)task->taskData)->providerId); + NS_VERIFY_NOT_NULL_V(prov); + NSTask * topTask = NSMakeTask(task->taskType, prov); + NS_VERIFY_NOT_NULL_V(topTask); + NSConsumerCommunicationTaskProcessing(topTask); + + NSRemoveProvider((NSProvider *)task->taskData); + NSOICFree(task); break; } case TASK_CONSUMER_REQ_SUBSCRIBE_CANCEL: { - NSProvider_internal * data = NSCopyProvider((NSProvider_internal *)task->taskData); + NSProvider_internal * data = + NSConsumerFindNSProvider(((NSProvider *)task->taskData)->providerId); NS_VERIFY_NOT_NULL_V(data); NSTask * conTask = NSMakeTask(TASK_CONSUMER_REQ_SUBSCRIBE_CANCEL, data); NS_VERIFY_NOT_NULL_V(conTask); - NSConsumerCommunicationTaskProcessing(task); - NSConsumerInternalTaskProcessing(conTask); + NSConsumerCommunicationTaskProcessing(conTask); + + data = NSConsumerFindNSProvider(((NSProvider *)task->taskData)->providerId); + NS_VERIFY_NOT_NULL_V(data); + NSTask * conTask2 = NSMakeTask(TASK_CONSUMER_REQ_SUBSCRIBE_CANCEL, data); + NSConsumerInternalTaskProcessing(conTask2); + + NSRemoveProvider((NSProvider *)task->taskData); + NSOICFree(task); break; } case TASK_RECV_SYNCINFO: -- 2.7.4