SF Bug Tracker [ 2392304 ] Memory leak in SSDP AdvertiseAndReply

Submitted: Ulrik ( ulsv_enea ) - 2008-12-05 08:24

	Valgrind reports a memory leak function in AdvertiseAndReply
	(ssdp/ssdp_server.c) in libupnp 1.6.6

	There are continue statements in many places in AdvertiseAndReply. In some
	of those error handling cases the variable nodelist is not free'ed before
	continuing to the next iteration. The next iteration will take care of
	free'ing the nodelist from the previous iteration in most cases, but not
	when breaking out of the for loop after the last element.

	I belive this memory leak can be solved by makeing sure that the rows

	ixmlNodeList_free( nodeList );
	nodeList = NULL;

	are always executed, also in the beginning of the last iteration when we
	found out that there are not more elements.

	==29110== at 0x4C21C16: malloc (vg_replace_malloc.c:149)
	==29110== by 0x5D8DE0E: ixmlNodeList_addToNodeList (nodeList.c:106)
	==29110== by 0x5D8B7E2: ixmlNode_getElementsByTagNameRecursive
	(node.c:1438)
	==29110== by 0x5D8E587: ixmlElement_getElementsByTagName
	(element.c:491)
	==29110== by 0x5B6C0F1: AdvertiseAndReply (ssdp_server.c:201)
	==29110== by 0x5B7AB74: UpnpSendAdvertisement (upnpapi.c:1495)



git-svn-id: https://pupnp.svn.sourceforge.net/svnroot/pupnp/branches/branch-1.6.x@518 119443c7-1b9e-41f8-b6fc-b9c35fce742c
This commit is contained in:
Marcelo Roberto Jimenez 2010-03-21 21:19:13 +00:00
parent 53d5e61b33
commit 712ed6d2ff
2 changed files with 289 additions and 298 deletions

@ -2,17 +2,48 @@
Version 1.6.7 Version 1.6.7
******************************************************************************* *******************************************************************************
2010-03-21 Marcelo Jimenez <mroberto(at)users.sourceforge.net>
* SF Bug Tracker [ 2392304 ] Memory leak in SSDP AdvertiseAndReply
Submitted: Ulrik ( ulsv_enea ) - 2008-12-05 08:24
Valgrind reports a memory leak function in AdvertiseAndReply
(ssdp/ssdp_server.c) in libupnp 1.6.6
There are continue statements in many places in AdvertiseAndReply. In some
of those error handling cases the variable nodelist is not free'ed before
continuing to the next iteration. The next iteration will take care of
free'ing the nodelist from the previous iteration in most cases, but not
when breaking out of the for loop after the last element.
I belive this memory leak can be solved by makeing sure that the rows
ixmlNodeList_free( nodeList );
nodeList = NULL;
are always executed, also in the beginning of the last iteration when we
found out that there are not more elements.
==29110== at 0x4C21C16: malloc (vg_replace_malloc.c:149)
==29110== by 0x5D8DE0E: ixmlNodeList_addToNodeList (nodeList.c:106)
==29110== by 0x5D8B7E2: ixmlNode_getElementsByTagNameRecursive
(node.c:1438)
==29110== by 0x5D8E587: ixmlElement_getElementsByTagName
(element.c:491)
==29110== by 0x5B6C0F1: AdvertiseAndReply (ssdp_server.c:201)
==29110== by 0x5B7AB74: UpnpSendAdvertisement (upnpapi.c:1495)
2010-03-21 Marcelo Jimenez <mroberto(at)users.sourceforge.net> 2010-03-21 Marcelo Jimenez <mroberto(at)users.sourceforge.net>
* libupnp and multi-flows scenario patch * libupnp and multi-flows scenario patch
Submited by Carlo Parata from STMicroelectronics. Submited by Carlo Parata from STMicroelectronics.
Hi Roberto and Nektarios,
after an analysis of the problem of libupnp with a multi-flows scenario, I Hi Roberto and Nektarios,
noticed that the only cause of the freezed system is the ThreadPool after an analysis of the problem of libupnp with a multi-flows scenario, I
management. There are not mutex problems. In practise, if all threads in the noticed that the only cause of the freezed system is the ThreadPool
thread pool are busy executing jobs, a new worker thread should be created if management. There are not mutex problems. In practise, if all threads in the
a job is scheduled (I inspired to tombupnp library). So I solved the problem thread pool are busy executing jobs, a new worker thread should be created if
with a little patch in threadutil library that you can find attached in this a job is scheduled (I inspired to tombupnp library). So I solved the problem
e-mail. I hope to have helped you. with a little patch in threadutil library that you can find attached in this
e-mail. I hope to have helped you.
2010-03-21 Marcelo Jimenez <mroberto(at)users.sourceforge.net> 2010-03-21 Marcelo Jimenez <mroberto(at)users.sourceforge.net>
* SF Patch Tracker [ 2964973 ] install: will not overwrite just-created * SF Patch Tracker [ 2964973 ] install: will not overwrite just-created

@ -91,309 +91,269 @@ struct SSDPSockArray {
* Returns: int * Returns: int
* UPNP_E_SUCCESS if successful else appropriate error * UPNP_E_SUCCESS if successful else appropriate error
***************************************************************************/ ***************************************************************************/
int AdvertiseAndReply( IN int AdFlag, int AdvertiseAndReply(
IN UpnpDevice_Handle Hnd, IN int AdFlag,
IN enum SsdpSearchType SearchType, IN UpnpDevice_Handle Hnd,
IN struct sockaddr_in *DestAddr, IN enum SsdpSearchType SearchType,
IN char *DeviceType, IN struct sockaddr_in *DestAddr,
IN char *DeviceUDN, IN char *DeviceType,
IN char *ServiceType, IN char *DeviceUDN,
int Exp ) IN char *ServiceType,
int Exp)
{ {
int i, int retVal = UPNP_E_SUCCESS;
j; int i;
int defaultExp = DEFAULT_MAXAGE; int j;
struct Handle_Info *SInfo = NULL; int defaultExp = DEFAULT_MAXAGE;
char UDNstr[100], struct Handle_Info *SInfo = NULL;
devType[100], char UDNstr[100];
servType[100]; char devType[100];
IXML_NodeList *nodeList = NULL; char servType[100];
IXML_NodeList *tmpNodeList = NULL; IXML_NodeList *nodeList = NULL;
IXML_Node *tmpNode = NULL; IXML_NodeList *tmpNodeList = NULL;
IXML_Node *tmpNode2 = NULL; IXML_Node *tmpNode = NULL;
IXML_Node *textNode = NULL; IXML_Node *tmpNode2 = NULL;
const DOMString tmpStr; IXML_Node *textNode = NULL;
char SERVER[200]; const DOMString tmpStr;
char SERVER[200];
const DOMString dbgStr;
const DOMString dbgStr; UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__, "Inside AdvertiseAndReply with AdFlag = %d\n", AdFlag);
"Inside AdvertiseAndReply with AdFlag = %d\n",
AdFlag );
// Use a read lock /* Use a read lock */
HandleReadLock(); HandleReadLock();
if( GetHandleInfo( Hnd, &SInfo ) != HND_DEVICE ) { if (GetHandleInfo(Hnd, &SInfo) != HND_DEVICE) {
HandleUnlock(); retVal = UPNP_E_INVALID_HANDLE;
return UPNP_E_INVALID_HANDLE; goto end_function;
}
defaultExp = SInfo->MaxAge;
//get server info
get_sdk_info( SERVER );
// parse the device list and send advertisements/replies
for( i = 0;; i++ ) {
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Entering new device list with i = %d\n\n", i );
tmpNode = ixmlNodeList_item( SInfo->DeviceList, i );
if( tmpNode == NULL ) {
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Exiting new device list with i = %d\n\n", i );
break;
}
dbgStr = ixmlNode_getNodeName( tmpNode );
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__,
"Extracting device type once for %s\n", dbgStr );
// extract device type
ixmlNodeList_free( nodeList );
nodeList = NULL;
nodeList = ixmlElement_getElementsByTagName(
( IXML_Element * ) tmpNode, "deviceType" );
if( nodeList == NULL ) {
continue;
}
dbgStr = ixmlNode_getNodeName( tmpNode );
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Extracting UDN for %s\n", dbgStr );
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Extracting device type\n" );
tmpNode2 = ixmlNodeList_item( nodeList, 0 );
if( tmpNode2 == NULL ) {
continue;
}
textNode = ixmlNode_getFirstChild( tmpNode2 );
if( textNode == NULL ) {
continue;
}
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Extracting device type \n" );
tmpStr = ixmlNode_getNodeValue( textNode );
if( tmpStr == NULL ) {
continue;
}
strcpy( devType, tmpStr );
if( devType == NULL ) {
continue;
}
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Extracting device type = %s\n", devType );
if( tmpNode == NULL ) {
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"TempNode is NULL\n" );
} }
dbgStr = ixmlNode_getNodeName( tmpNode );
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Extracting UDN for %s\n", dbgStr );
// extract UDN
ixmlNodeList_free( nodeList );
nodeList = NULL;
nodeList = ixmlElement_getElementsByTagName( ( IXML_Element * )
tmpNode, "UDN" );
if( nodeList == NULL ) {
UpnpPrintf( UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!!!\n" );
continue;
}
tmpNode2 = ixmlNodeList_item( nodeList, 0 );
if( tmpNode2 == NULL ) {
UpnpPrintf( UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!!!\n" );
continue;
}
textNode = ixmlNode_getFirstChild( tmpNode2 );
if( textNode == NULL ) {
UpnpPrintf( UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!!!\n" );
continue;
}
tmpStr = ixmlNode_getNodeValue( textNode );
if( tmpStr == NULL ) {
UpnpPrintf( UPNP_CRITICAL, API, __FILE__, __LINE__,
"UDN not found!!!!\n" );
continue;
}
strcpy( UDNstr, tmpStr );
if( UDNstr == NULL ) {
continue;
}
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, defaultExp = SInfo->MaxAge;
"Sending UDNStr = %s \n", UDNstr );
if( AdFlag ) {
// send the device advertisement
if( AdFlag == 1 ) {
DeviceAdvertisement( devType, i == 0,
UDNstr, SInfo->DescURL, Exp );
} else { // AdFlag == -1
DeviceShutdown( devType, i == 0, UDNstr,
SERVER, SInfo->DescURL, Exp );
}
} else {
switch ( SearchType ) {
case SSDP_ALL: /* get server info */
DeviceReply( DestAddr, get_sdk_info(SERVER);
devType, i == 0,
UDNstr, SInfo->DescURL, defaultExp );
break;
case SSDP_ROOTDEVICE: /* parse the device list and send advertisements/replies */
if( i == 0 ) { for (i = 0;; i++) {
SendReply( DestAddr, devType, 1, UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
UDNstr, SInfo->DescURL, defaultExp, 0 ); "Entering new device list with i = %d\n\n", i);
} tmpNode = ixmlNodeList_item(SInfo->DeviceList, i);
break; if (!tmpNode) {
case SSDP_DEVICEUDN: UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
{ "Exiting new device list with i = %d\n\n", i);
if( DeviceUDN != NULL && strlen( DeviceUDN ) != 0 ) { break;
if( strcasecmp( DeviceUDN, UDNstr ) ) { }
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, dbgStr = ixmlNode_getNodeName(tmpNode);
"DeviceUDN=%s and search "
"UDN=%s did not match\n", UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
UDNstr, DeviceUDN ); "Extracting device type once for %s\n", dbgStr);
break; ixmlNodeList_free(nodeList);
} else { nodeList = ixmlElement_getElementsByTagName(
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, (IXML_Element *)tmpNode, "deviceType");
"DeviceUDN=%s and search " if (!nodeList) continue;
"UDN=%s MATCH\n", UDNstr,
DeviceUDN ); UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
SendReply( DestAddr, devType, 0, "Extracting UDN for %s\n", dbgStr);
UDNstr, SInfo->DescURL, dbgStr = ixmlNode_getNodeName(tmpNode);
defaultExp, 0 );
break; UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
} "Extracting device type\n");
} tmpNode2 = ixmlNodeList_item(nodeList, 0);
} if (!tmpNode2) continue;
case SSDP_DEVICETYPE:
{ textNode = ixmlNode_getFirstChild(tmpNode2);
if( !strncasecmp if (!textNode) continue;
( DeviceType, devType,
strlen( DeviceType ) ) ) { UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, "Extracting device type \n");
"DeviceType=%s and search devType=%s MATCH\n", tmpStr = ixmlNode_getNodeValue(textNode);
devType, DeviceType ); if (!tmpStr) continue;
SendReply( DestAddr, devType, 0, UDNstr,
SInfo->DescURL, defaultExp, 1 ); strcpy(devType, tmpStr);
} else { UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, "Extracting device type = %s\n", devType);
"DeviceType=%s and search devType=%s" if (!tmpNode) {
" DID NOT MATCH\n", UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
devType, DeviceType ); "TempNode is NULL\n");
}
dbgStr = ixmlNode_getNodeName(tmpNode);
UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
"Extracting UDN for %s\n", dbgStr);
ixmlNodeList_free(nodeList);
nodeList = ixmlElement_getElementsByTagName(
(IXML_Element *)tmpNode, "UDN");
if (!nodeList) {
UpnpPrintf(UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!\n");
continue;
}
tmpNode2 = ixmlNodeList_item(nodeList, 0);
if (!tmpNode2) {
UpnpPrintf(UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!\n");
continue;
}
textNode = ixmlNode_getFirstChild(tmpNode2);
if (!textNode) {
UpnpPrintf(UPNP_CRITICAL, API, __FILE__,
__LINE__, "UDN not found!\n");
continue;
}
tmpStr = ixmlNode_getNodeValue(textNode);
if (!tmpStr) {
UpnpPrintf(UPNP_CRITICAL, API, __FILE__, __LINE__,
"UDN not found!\n");
continue;
}
strcpy(UDNstr, tmpStr);
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"Sending UDNStr = %s \n", UDNstr);
if (AdFlag) {
/* send the device advertisement */
if (AdFlag == 1) {
DeviceAdvertisement(devType, i == 0,
UDNstr, SInfo->DescURL, Exp);
} else {
/* AdFlag == -1 */
DeviceShutdown(devType, i == 0, UDNstr,
SERVER, SInfo->DescURL, Exp);
} }
break; } else {
} switch (SearchType) {
default: case SSDP_ALL:
break; DeviceReply(DestAddr, devType, i == 0,
} UDNstr, SInfo->DescURL, defaultExp);
} break;
// send service advertisements for services corresponding case SSDP_ROOTDEVICE:
// to the same device if (i == 0) {
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, SendReply(DestAddr, devType, 1,
"Sending service Advertisement\n" ); UDNstr, SInfo->DescURL, defaultExp, 0);
}
break;
case SSDP_DEVICEUDN: {
if (DeviceUDN && strlen(DeviceUDN) != 0) {
if (strcasecmp(DeviceUDN, UDNstr)) {
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"DeviceUDN=%s and search "
"UDN=%s did not match\n",
UDNstr, DeviceUDN);
break;
} else {
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"DeviceUDN=%s and search "
"UDN=%s MATCH\n", UDNstr,
DeviceUDN);
SendReply(DestAddr, devType, 0,
UDNstr, SInfo->DescURL,
defaultExp, 0);
break;
}
}
}
case SSDP_DEVICETYPE: {
if (!strncasecmp(DeviceType, devType, strlen(DeviceType))) {
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"DeviceType=%s and search devType=%s MATCH\n",
devType, DeviceType);
SendReply(DestAddr, devType, 0, UDNstr,
SInfo->DescURL, defaultExp, 1);
} else {
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"DeviceType=%s and search devType=%s"
" DID NOT MATCH\n",
devType, DeviceType);
}
break;
}
default:
break;
}
}
/* send service advertisements for services corresponding
* to the same device */
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"Sending service Advertisement\n");
tmpNode = ixmlNodeList_item( SInfo->ServiceList, i ); tmpNode = ixmlNodeList_item(SInfo->ServiceList, i);
if( tmpNode == NULL ) { if (!tmpNode) continue;
continue; ixmlNodeList_free(nodeList);
} nodeList = ixmlElement_getElementsByTagName(
ixmlNodeList_free( nodeList ); (IXML_Element *)tmpNode, "service");
nodeList = NULL; if (!nodeList) {
nodeList = ixmlElement_getElementsByTagName( ( IXML_Element * ) UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__,
tmpNode, "service" ); "Service not found 3\n" );
if( nodeList == NULL ) { continue;
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, }
"Service not found 3\n" ); for (j = 0;; j++) {
continue; tmpNode = ixmlNodeList_item(nodeList, j);
} if (!tmpNode) {
for( j = 0;; j++ ) { break;
tmpNode = ixmlNodeList_item( nodeList, j ); }
if( tmpNode == NULL ) { ixmlNodeList_free(tmpNodeList);
break; tmpNodeList = ixmlElement_getElementsByTagName(
} (IXML_Element *)tmpNode, "serviceType");
if (!tmpNodeList) {
UpnpPrintf(UPNP_CRITICAL, API, __FILE__, __LINE__,
"ServiceType not found \n");
continue;
}
tmpNode2 = ixmlNodeList_item(tmpNodeList, 0);
if (!tmpNode2) continue;
textNode = ixmlNode_getFirstChild(tmpNode2);
if (!textNode) continue;
/* servType is of format Servicetype:ServiceVersion */
tmpStr = ixmlNode_getNodeValue(textNode);
if (!tmpStr) continue;
strcpy(servType, tmpStr);
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__,
"ServiceType = %s\n", servType);
if (AdFlag) {
if (AdFlag == 1) {
ServiceAdvertisement(UDNstr, servType,
SInfo->DescURL, Exp);
} else {
/* AdFlag == -1 */
ServiceShutdown(UDNstr, servType,
SInfo->DescURL, Exp);
}
} else {
switch (SearchType) {
case SSDP_ALL:
ServiceReply(DestAddr, servType,
UDNstr, SInfo->DescURL,
defaultExp);
break;
case SSDP_SERVICE:
if (ServiceType) {
if (!strncasecmp(ServiceType, servType, strlen(ServiceType))) {
ServiceReply(DestAddr, servType,
UDNstr, SInfo->DescURL, defaultExp);
}
}
break;
default:
break;
}
}
}
ixmlNodeList_free(tmpNodeList);
tmpNodeList = NULL;
ixmlNodeList_free(nodeList);
nodeList = NULL;
}
ixmlNodeList_free( tmpNodeList ); end_function:
tmpNodeList = NULL; ixmlNodeList_free(tmpNodeList);
tmpNodeList = ixmlElement_getElementsByTagName( ixmlNodeList_free(nodeList);
( IXML_Element *)tmpNode, "serviceType" );
if( tmpNodeList == NULL ) {
UpnpPrintf( UPNP_CRITICAL, API, __FILE__, __LINE__,
"ServiceType not found \n" );
continue;
}
tmpNode2 = ixmlNodeList_item( tmpNodeList, 0 );
if( tmpNode2 == NULL ) {
continue;
}
textNode = ixmlNode_getFirstChild( tmpNode2 );
if( textNode == NULL ) {
continue;
}
// servType is of format Servicetype:ServiceVersion
tmpStr = ixmlNode_getNodeValue( textNode );
if( tmpStr == NULL ) {
continue;
}
strcpy( servType, tmpStr );
if( servType == NULL ) {
continue;
}
UpnpPrintf( UPNP_INFO, API, __FILE__, __LINE__, UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__,
"ServiceType = %s\n", servType ); "Exiting AdvertiseAndReply.\n");
if( AdFlag ) {
if( AdFlag == 1 ) {
ServiceAdvertisement( UDNstr, servType,
SInfo->DescURL, Exp );
} else { // AdFlag == -1
ServiceShutdown( UDNstr, servType,
SInfo->DescURL, Exp );
}
} else {
switch ( SearchType ) {
case SSDP_ALL:
ServiceReply( DestAddr, servType,
UDNstr, SInfo->DescURL,
defaultExp );
break;
case SSDP_SERVICE:
if( ServiceType != NULL ) {
if( !strncasecmp( ServiceType,
servType,
strlen( ServiceType ) ) ) {
ServiceReply( DestAddr, servType,
UDNstr, SInfo->DescURL,
defaultExp );
}
}
break;
default:
break;
} // switch(SearchType)
} HandleUnlock();
}
ixmlNodeList_free( tmpNodeList );
tmpNodeList = NULL;
ixmlNodeList_free( nodeList );
nodeList = NULL;
}
UpnpPrintf( UPNP_ALL, API, __FILE__, __LINE__,
"Exiting AdvertiseAndReply : \n" );
HandleUnlock(); return retVal;
}
return UPNP_E_SUCCESS;
} /****************** End of AdvertiseAndReply *********************/
#endif /* EXCLUDE_SSDP == 0 */ #endif /* EXCLUDE_SSDP == 0 */
#endif /* INCLUDE_DEVICE_APIS */ #endif /* INCLUDE_DEVICE_APIS */