Skip to content

Commit

Permalink
Fix Memory leak if libzt events aren't consumed by user app
Browse files Browse the repository at this point in the history 
If arg is not enqueued by Events, then treat as if ownership has NOT
been transferred and caller of Events->enqueue is responsible for freeing
  • Loading branch information
@bostick
bostick committed Aug 21, 2023
1 parent f5ffc06 commit 4ec2bcd
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 17 deletions.
26 changes: 15 additions & 11 deletions src/Events.cpp
View file
Expand Up @@ -93,11 +93,18 @@ void Events::run()
}
}

void Events::enqueue(unsigned int event_code, const void* arg, int len)
bool Events::enqueue(unsigned int event_code, const void* arg, int len)
{
if (! _enabled) {
return;
return false;
}
if (_callbackMsgQueue.size_approx() > 1024) {
/* Rate-limit number of events. This value should only grow if the
user application isn't returning from the event handler in a timely manner.
For most applications it should hover around 1 to 2 */
return false;
}

zts_event_msg_t* msg = new zts_event_msg_t();
msg->event_code = event_code;

Expand Down Expand Up @@ -132,15 +139,12 @@ void Events::enqueue(unsigned int event_code, const void* arg, int len)
msg->cache = (void*)arg;
msg->len = len;
}
if (msg && _callbackMsgQueue.size_approx() > 1024) {
/* Rate-limit number of events. This value should only grow if the
user application isn't returning from the event handler in a timely manner.
For most applications it should hover around 1 to 2 */
destroy(msg);
}
else {
_callbackMsgQueue.enqueue(msg);
}

//
// ownership of arg is now transferred
//
_callbackMsgQueue.enqueue(msg);
return true;
}

void Events::destroy(zts_event_msg_t* msg)
Expand Down
6 changes: 5 additions & 1 deletion src/Events.hpp
View file
Expand Up @@ -115,8 +115,12 @@ class Events {

/**
* Enqueue an event to be sent to the user application
*
* Returns true if arg was enqueued.
* If enqueued, then ownership of arg has been transferred.
* If NOT enqueued, then ownership of arg has NOT been transferred.
*/
void enqueue(unsigned int event_code, const void* arg, int len = 0);
bool enqueue(unsigned int event_code, const void* arg, int len = 0);

/**
* Send callback message to user application
Expand Down
71 changes: 66 additions & 5 deletions src/NodeService.cpp
View file
Expand Up @@ -957,13 +957,17 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u

void* objptr = NULL;

zts_node_info_t* nd;
zts_net_info_t* nt;
zts_peer_info_t* pr;

switch (zt_event_code) {
case ZTS_EVENT_NODE_UP:
case ZTS_EVENT_NODE_ONLINE:
case ZTS_EVENT_NODE_OFFLINE:
case ZTS_EVENT_NODE_DOWN:
case ZTS_EVENT_NODE_FATAL_ERROR: {
zts_node_info_t* nd = new zts_node_info_t;
nd = new zts_node_info_t;
nd->node_id = _nodeId;
nd->ver_major = ZEROTIER_ONE_VERSION_MAJOR;
nd->ver_minor = ZEROTIER_ONE_VERSION_MINOR;
Expand All @@ -980,7 +984,7 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
case ZTS_EVENT_NETWORK_ACCESS_DENIED:
case ZTS_EVENT_NETWORK_DOWN: {
NetworkState* ns = (NetworkState*)obj;
zts_net_info_t* nt = new zts_net_info_t();
nt = new zts_net_info_t();
nt->net_id = ns->config.nwid;
objptr = (void*)nt;
break;
Expand All @@ -990,7 +994,7 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
case ZTS_EVENT_NETWORK_READY_IP6:
case ZTS_EVENT_NETWORK_OK: {
NetworkState* ns = (NetworkState*)obj;
zts_net_info_t* nt = new zts_net_info_t();
nt = new zts_net_info_t();
nt->net_id = ns->config.nwid;
nt->mac = ns->config.mac;
strncpy(nt->name, ns->config.name, sizeof(ns->config.name));
Expand Down Expand Up @@ -1051,7 +1055,7 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
case ZTS_EVENT_PEER_UNREACHABLE:
case ZTS_EVENT_PEER_PATH_DISCOVERED:
case ZTS_EVENT_PEER_PATH_DEAD: {
zts_peer_info_t* pr = new zts_peer_info_t();
pr = new zts_peer_info_t();
ZT_Peer* peer = (ZT_Peer*)obj;
memcpy(pr, peer, sizeof(zts_peer_info_t));
for (unsigned int j = 0; j < peer->pathCount; j++) {
Expand All @@ -1067,7 +1071,64 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
// Send event

if (objptr) {
_events->enqueue(zt_event_code, objptr, len);
if (!_events->enqueue(zt_event_code, objptr, len)) {
//
// ownership of objptr was NOT transferred, so delete any news from above
//
switch (zt_event_code) {
case ZTS_EVENT_NODE_UP:
case ZTS_EVENT_NODE_ONLINE:
case ZTS_EVENT_NODE_OFFLINE:
case ZTS_EVENT_NODE_DOWN:
case ZTS_EVENT_NODE_FATAL_ERROR: {
delete nd;
break;
}
case ZTS_EVENT_NETWORK_NOT_FOUND:
case ZTS_EVENT_NETWORK_CLIENT_TOO_OLD:
case ZTS_EVENT_NETWORK_REQ_CONFIG:
case ZTS_EVENT_NETWORK_ACCESS_DENIED:
case ZTS_EVENT_NETWORK_DOWN: {
delete nt;
break;
}
case ZTS_EVENT_NETWORK_UPDATE:
case ZTS_EVENT_NETWORK_READY_IP4:
case ZTS_EVENT_NETWORK_READY_IP6:
case ZTS_EVENT_NETWORK_OK: {
delete nt;
break;
}
case ZTS_EVENT_ADDR_ADDED_IP4:
break;
case ZTS_EVENT_ADDR_ADDED_IP6:
break;
case ZTS_EVENT_ADDR_REMOVED_IP4:
break;
case ZTS_EVENT_ADDR_REMOVED_IP6:
break;
case ZTS_EVENT_STORE_IDENTITY_PUBLIC:
break;
case ZTS_EVENT_STORE_IDENTITY_SECRET:
break;
case ZTS_EVENT_STORE_PLANET:
break;
case ZTS_EVENT_STORE_PEER:
break;
case ZTS_EVENT_STORE_NETWORK:
break;
case ZTS_EVENT_PEER_DIRECT:
case ZTS_EVENT_PEER_RELAY:
case ZTS_EVENT_PEER_UNREACHABLE:
case ZTS_EVENT_PEER_PATH_DISCOVERED:
case ZTS_EVENT_PEER_PATH_DEAD: {
delete pr;
break;
}
default:
break;
}
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/VirtualTap.cpp
View file
Expand Up @@ -312,6 +312,10 @@ static void zts_main_lwip_driver_loop(void* arg)
zts_util_delay(LWIP_DRIVER_LOOP_INTERVAL);
}
_has_exited = true;

//
// no need to check if event was enqueued since NULL is being passed
//
zts_events->enqueue(ZTS_EVENT_STACK_DOWN, NULL);
}

Expand Down

0 comments on commit 4ec2bcd

Please sign in to comment.