Bugfix for hang in VirtualTap after calling getRoutes() and attempting to leave(), bugfix for potential memory leak if packet was rejected from stack

include/VirtualTap.hpp

@@ -57,7 +57,8 @@ namespace ZeroTier {
 class Mutex;
 
 /**
- * emulates an Ethernet tap device
+ * A virtual tap device. The ZeroTier core service creates one of these for each
+ * virtual network joined. It will be destroyed upon leave().
  */
 class VirtualTap
 {
@@ -210,9 +211,9 @@ public:
 
 	std::vector<MulticastGroup> _multicastGroups;
 	Mutex _multicastGroups_m;
-	Mutex _ips_m, _tcpconns_m, _rx_buf_m, _close_m;
+	Mutex _ips_m;
 
-	struct zts_network_details nd;
+	//struct zts_network_details nd;
 
 	/*
 	 * Timestamp of last run of housekeeping

src/VirtualTap.cpp

@@ -51,6 +51,10 @@ namespace ZeroTier {
 extern OneService *zt1Service;
 extern void (*_userCallbackFunc)(uint64_t, int);
 
+/**
+ * A virtual tap device. The ZeroTier core service creates one of these for each
+ * virtual network joined. It will be destroyed upon leave().
+ */
 VirtualTap::VirtualTap(
 	const char *homePath,
 	const MAC &mac,
@@ -247,72 +251,68 @@ void VirtualTap::threadMain()
 
 void VirtualTap::Housekeeping()
 {
-	Mutex::Lock _l(_tcpconns_m);
+/*
+	Mutex::Lock _l(_tap_m);
 	OneService *service = ((OneService *)zt1Service);
 	if (!service) {
 		return;
 	}
 	nd.num_routes = ZTS_MAX_NETWORK_ROUTES;
 	service->getRoutes(this->_nwid, (ZT_VirtualNetworkRoute*)&(nd.routes)[0], &(nd.num_routes));
-
-/*
-
-			
-			InetAddress target_addr;
-			InetAddress via_addr;
-			InetAddress null_addr;
-			InetAddress nm;
-			null_addr.fromString("");
-			bool found;
-			char ipbuf[INET6_ADDRSTRLEN], ipbuf2[INET6_ADDRSTRLEN], ipbuf3[INET6_ADDRSTRLEN];
 */
 
-
+/*			
-			// TODO: Rework this when we have time
+	InetAddress target_addr;
-			// check if pushed route exists in tap (add)
+	InetAddress via_addr;
-			/*
+	InetAddress null_addr;
-			for (int i=0; i<ZT_MAX_NETWORK_ROUTES; i++) {
+	InetAddress nm;
-				found = false;
+	null_addr.fromString("");
-				target_addr = managed_routes->at(i).target;
+	bool found;
-				via_addr = managed_routes->at(i).via;
+	char ipbuf[INET6_ADDRSTRLEN], ipbuf2[INET6_ADDRSTRLEN], ipbuf3[INET6_ADDRSTRLEN];
-				nm = target_addr.netmask();
+
-				for (size_t j=0; j<routes.size(); j++) {
+	// TODO: Rework this when we have time
-					if (via_addr.ipsEqual(null_addr) || target_addr.ipsEqual(null_addr)) {
+	// check if pushed route exists in tap (add)
-						found=true;
+	/*
-						continue;
+	for (int i=0; i<ZT_MAX_NETWORK_ROUTES; i++) {
-					}
+		found = false;
-					if (routes[j].first.ipsEqual(target_addr) && routes[j].second.ipsEqual(nm)) {
+		target_addr = managed_routes->at(i).target;
-						found=true;
+		via_addr = managed_routes->at(i).via;
-					}
+		nm = target_addr.netmask();
-				}
+		for (size_t j=0; j<routes.size(); j++) {
-				if (found == false) {
+			if (via_addr.ipsEqual(null_addr) || target_addr.ipsEqual(null_addr)) {
-					if (via_addr.ipsEqual(null_addr) == false) {
+				found=true;
-						DEBUG_INFO("adding route <target=%s, nm=%s, via=%s>", target_addr.toString(ipbuf), nm.toString(ipbuf2), via_addr.toString(ipbuf3));
+				continue;
-						routes.push_back(std::pair<InetAddress,InetAddress>(target_addr, nm));
+			}
-						routeAdd(target_addr, nm, via_addr);
+			if (routes[j].first.ipsEqual(target_addr) && routes[j].second.ipsEqual(nm)) {
-					}
+				found=true;
-				}
 			}
-			// check if route exists in tap but not in pushed routes (remove)
+		}
-			for (size_t i=0; i<routes.size(); i++) {
+		if (found == false) {
-				found = false;
+			if (via_addr.ipsEqual(null_addr) == false) {
-				for (int j=0; j<ZT_MAX_NETWORK_ROUTES; j++) {
+				DEBUG_INFO("adding route <target=%s, nm=%s, via=%s>", target_addr.toString(ipbuf), nm.toString(ipbuf2), via_addr.toString(ipbuf3));
-					target_addr = managed_routes->at(j).target;
+				routes.push_back(std::pair<InetAddress,InetAddress>(target_addr, nm));
-					via_addr = managed_routes->at(j).via;
+				routeAdd(target_addr, nm, via_addr);
-					nm = target_addr.netmask();
+			}
-					if (routes[i].first.ipsEqual(target_addr) && routes[i].second.ipsEqual(nm)) {
+		}
-						found=true;
+	}
-					}
+	// check if route exists in tap but not in pushed routes (remove)
-				}
+	for (size_t i=0; i<routes.size(); i++) {
-				if (found == false) {
+		found = false;
-					DEBUG_INFO("removing route to <%s,%s>", routes[i].first.toString(ipbuf), routes[i].second.toString(ipbuf2));
+		for (int j=0; j<ZT_MAX_NETWORK_ROUTES; j++) {
-					routes.erase(routes.begin() + i);
+			target_addr = managed_routes->at(j).target;
-					routeDelete(routes[i].first, routes[i].second);
+			via_addr = managed_routes->at(j).via;
-				}
+			nm = target_addr.netmask();
+			if (routes[i].first.ipsEqual(target_addr) && routes[i].second.ipsEqual(nm)) {
+				found=true;
 			}
-			*/
+		}
-		//}
+		if (found == false) {
-		// TODO: Clean up VirtualSocket objects
+			DEBUG_INFO("removing route to <%s,%s>", routes[i].first.toString(ipbuf), routes[i].second.toString(ipbuf2));
+			routes.erase(routes.begin() + i);
+			routeDelete(routes[i].first, routes[i].second);
+		}
+	}
+*/
 }
 
 //////////////////////////////////////////////////////////////////////////////

src/lwIP.cpp

@@ -108,6 +108,7 @@ void my_tcpip_callback(void *arg)
 	if (main_loop_exited) {
 		return;
 	}
+	// stats_display();
 	err_t err = ERR_OK;
 	int loop_score = LWIP_FRAMES_HANDLED_PER_CORE_CALL; // max num of packets to read per polling call
 	// TODO: Optimize (use Ringbuffer)
@@ -131,8 +132,10 @@ void my_tcpip_callback(void *arg)
 				for (size_t i=0; i<lwip_netifs.size(); i++) {
 					if (lwip_netifs[i]->output_ip6 && 
 						lwip_netifs[i]->output_ip6 == ethip6_output) {
+						// TODO: Check prefix match?
 						if ((err = lwip_netifs[i]->input(p, lwip_netifs[i])) != ERR_OK) {
 							DEBUG_ERROR("packet input error (ipv6, p=%p, netif=%p)=%d", p, &lwip_netifs[i], err);
+							pbuf_free(p);
 						}
 						break;
 					}
@@ -147,6 +150,7 @@ void my_tcpip_callback(void *arg)
 							ip4_addr_isbroadcast_u32(iphdr->dest.addr, lwip_netifs[i])) {
 							if ((err = lwip_netifs[i]->input(p, lwip_netifs[i])) != ERR_OK) {
 								DEBUG_ERROR("packet input error (ipv4, p=%p, netif=%p)=%d", p, &lwip_netifs[i], err);
+								pbuf_free(p);
 							}
 							break;
 						}
@@ -159,6 +163,7 @@ void my_tcpip_callback(void *arg)
 						//pbuf_ref(p);
 						if ((err = lwip_netifs[i]->input(p, lwip_netifs[i])) != ERR_OK) {
 							DEBUG_ERROR("packet input error (arp, p=%p, netif=%p)=%d", p, &lwip_netifs[i], err);
+							pbuf_free(p);
 						}
 						break;
 					}
@@ -166,9 +171,10 @@ void my_tcpip_callback(void *arg)
 				break;
 			} break;
 			default:
-				DEBUG_INFO("unhandled packet type (p=%p)=%d", p, err);
+				DEBUG_INFO("unhandled packet type (p=%p)", p);
 				break;
 		}
+		//
 		p = NULL;
 		loop_score--;
 	}
@@ -377,8 +383,6 @@ void lwip_eth_rx(ZeroTier::VirtualTap *tap, const ZeroTier::MAC &from, const Zer
 		return;
 	}
 	rx_queue.push(p);
-	DEBUG_INFO("GOT packet=%p", p);
-	//pbuf_ref(p);
 	_rx_input_lock_m.unlock();
 }