libzt icon indicating copy to clipboard operation
libzt copied to clipboard

Race conditions in VirtualTap::put()

Open StephenCWills opened this issue 4 years ago • 0 comments

By abusing the logic for LWIP_ASSERT_CORE_LOCKED() on Windows, I was able to confirm that VirtualTap::put() is sometimes called inside the tcpip core lock and sometimes outside. Adding lock statements around netif->input() calls as I did in #162 resolves the race conditions in cases where the lock is not taken, but causes a deadlock on Linux systems because the tcpip core lock uses a mutex that is not reentrant.

It seems that when we first implemented ZeroTier in DevilutionX, we worked around this by switching to a reentrant mutex. https://github.com/diasurgical/lwip-contrib/commit/1f9e26e221a41542563834222c4ec8399be1908f


In this case, the tcpip core lock was not taken which leads to the race condition.

 	ucrtbased.dll!00007ffa4fdf7475()	Unknown
 	ucrtbased.dll!00007ffa4fdf7613()	Unknown
 	ucrtbased.dll!00007ffa4fe0d86d()	Unknown
>	devilutionx.exe!sys_check_core_locking() Line 506	C
 	devilutionx.exe!ZeroTier::zts_lwip_eth_rx(ZeroTier::VirtualTap * tap, const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, const void * data, unsigned int len) Line 475	C++
 	devilutionx.exe!ZeroTier::VirtualTap::put(const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, const void * data, unsigned int len) Line 214	C++
 	devilutionx.exe!ZeroTier::NodeService::nodeVirtualNetworkFrameFunction(unsigned __int64 net_id, void * * nuptr, unsigned __int64 sourceMac, unsigned __int64 destMac, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 1487	C++
 	devilutionx.exe!ZeroTier::SnodeVirtualNetworkFrameFunction(void * node, void * uptr, void * tptr, unsigned __int64 net_id, void * * nuptr, unsigned __int64 sourceMac, unsigned __int64 destMac, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 122	C++
 	devilutionx.exe!ZeroTier::Node::putFrame(void * tPtr, unsigned __int64 nwid, void * * nuptr, const ZeroTier::MAC & source, const ZeroTier::MAC & dest, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 134	C++
 	devilutionx.exe!ZeroTier::IncomingPacket::_doMULTICAST_FRAME(const ZeroTier::RuntimeEnvironment * RR, void * tPtr, const ZeroTier::SharedPtr<ZeroTier::Peer> & peer) Line 1177	C++
 	devilutionx.exe!ZeroTier::IncomingPacket::tryDecode(const ZeroTier::RuntimeEnvironment * RR, void * tPtr, int flowId) Line 104	C++
 	devilutionx.exe!ZeroTier::Switch::onRemotePacket(void * tPtr, const __int64 localSocket, const ZeroTier::InetAddress & fromAddr, const void * data, unsigned int len) Line 259	C++
 	devilutionx.exe!ZeroTier::Node::processWirePacket(void * tptr, __int64 now, __int64 localSocket, const sockaddr_storage * remoteAddress, const void * packetData, unsigned int packetLength, volatile __int64 * nextBackgroundTaskDeadline) Line 165	C++
 	devilutionx.exe!ZeroTier::NodeService::phyOnDatagram(void * sock, void * * uptr, const sockaddr * localAddr, const sockaddr * from, void * data, unsigned long len) Line 618	C++
 	devilutionx.exe!ZeroTier::Phy<ZeroTier::NodeService *>::poll(unsigned long timeout) Line 987	C++
 	devilutionx.exe!ZeroTier::NodeService::run() Line 478	C++
 	devilutionx.exe!_runNodeService(void * arg) Line 515	C++
 	[External Code]	

In this case, the tcpip core lock is taken by tcpip_send_msg_wait_sem() so this would lead to the deadlock.

 	ucrtbased.dll!00007ffa4fdf7475()	Unknown
 	ucrtbased.dll!00007ffa4fdf7613()	Unknown
 	ucrtbased.dll!00007ffa4fe0d86d()	Unknown
>	devilutionx.exe!sys_check_core_locking() Line 506	C
 	devilutionx.exe!ZeroTier::zts_lwip_eth_rx(ZeroTier::VirtualTap * tap, const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, const void * data, unsigned int len) Line 475	C++
 	devilutionx.exe!ZeroTier::VirtualTap::put(const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, const void * data, unsigned int len) Line 214	C++
 	devilutionx.exe!ZeroTier::NodeService::nodeVirtualNetworkFrameFunction(unsigned __int64 net_id, void * * nuptr, unsigned __int64 sourceMac, unsigned __int64 destMac, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 1487	C++
 	devilutionx.exe!ZeroTier::SnodeVirtualNetworkFrameFunction(void * node, void * uptr, void * tptr, unsigned __int64 net_id, void * * nuptr, unsigned __int64 sourceMac, unsigned __int64 destMac, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 122	C++
 	devilutionx.exe!ZeroTier::Node::putFrame(void * tPtr, unsigned __int64 nwid, void * * nuptr, const ZeroTier::MAC & source, const ZeroTier::MAC & dest, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 134	C++
 	devilutionx.exe!ZeroTier::Switch::onLocalEthernet(void * tPtr, const ZeroTier::SharedPtr<ZeroTier::Network> & network, const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 460	C++
 	devilutionx.exe!ZeroTier::Node::processVirtualNetworkFrame(void * tptr, __int64 now, unsigned __int64 nwid, unsigned __int64 sourceMac, unsigned __int64 destMac, unsigned int etherType, unsigned int vlanId, const void * frameData, unsigned int frameLength, volatile __int64 * nextBackgroundTaskDeadline) Line 184	C++
 	devilutionx.exe!ZeroTier::NodeService::tapFrameHandler(unsigned __int64 net_id, const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 1594	C++
 	devilutionx.exe!ZeroTier::StapFrameHandler(void * uptr, void * tptr, unsigned __int64 net_id, const ZeroTier::MAC & from, const ZeroTier::MAC & to, unsigned int etherType, unsigned int vlanId, const void * data, unsigned int len) Line 163	C++
 	devilutionx.exe!ZeroTier::zts_lwip_eth_tx(netif * n, pbuf * p) Line 412	C++
 	devilutionx.exe!ethernet_output(netif * netif, pbuf * p, const eth_addr * src, const eth_addr * dst, unsigned short eth_type) Line 312	C
 	devilutionx.exe!ethip6_output(netif * netif, pbuf * q, const ip6_addr * ip6addr) Line 101	C
 	devilutionx.exe!ip6_output_if_src(pbuf * p, const ip6_addr * src, const ip6_addr * dest, unsigned char hl, unsigned char tc, unsigned char nexth, netif * netif) Line 1270	C
 	devilutionx.exe!ip6_output_if(pbuf * p, const ip6_addr * src, const ip6_addr * dest, unsigned char hl, unsigned char tc, unsigned char nexth, netif * netif) Line 1166	C
 	devilutionx.exe!nd6_send_ns(netif * netif, const ip6_addr * target_addr, unsigned char flags) Line 1243	C
 	devilutionx.exe!nd6_send_neighbor_cache_probe(nd6_neighbor_cache_entry * entry, unsigned char flags) Line 1171	C
 	devilutionx.exe!nd6_get_next_hop_entry(const ip6_addr * ip6addr, netif * netif) Line 2049	C
 	devilutionx.exe!nd6_get_next_hop_addr_or_queue(netif * netif, pbuf * q, const ip6_addr * ip6addr, const unsigned char * * hwaddrp) Line 2263	C
 	devilutionx.exe!ethip6_output(netif * netif, pbuf * q, const ip6_addr * ip6addr) Line 108	C
 	devilutionx.exe!ip6_output_if_src(pbuf * p, const ip6_addr * src, const ip6_addr * dest, unsigned char hl, unsigned char tc, unsigned char nexth, netif * netif) Line 1270	C
 	devilutionx.exe!udp_sendto_if_src(udp_pcb * pcb, pbuf * p, const ip_addr * dst_ip, unsigned short dst_port, netif * netif, const ip_addr * src_ip) Line 893	C
 	devilutionx.exe!udp_sendto_if(udp_pcb * pcb, pbuf * p, const ip_addr * dst_ip, unsigned short dst_port, netif * netif) Line 694	C
 	devilutionx.exe!udp_sendto(udp_pcb * pcb, pbuf * p, const ip_addr * dst_ip, unsigned short dst_port) Line 601	C
 	devilutionx.exe!lwip_netconn_do_send(void * m) Line 1567	C
 	devilutionx.exe!tcpip_send_msg_wait_sem(void(*)(void *) fn, void * apimsg, _sys_sem * sem) Line 443	C
 	devilutionx.exe!netconn_apimsg(void(*)(void *) fn, api_msg * apimsg) Line 131	C
 	devilutionx.exe!netconn_send(netconn * conn, netbuf * buf) Line 953	C
 	devilutionx.exe!lwip_sendto(int s, const void * data, unsigned __int64 size, int flags, const sockaddr * to, unsigned int tolen) Line 1675	C
 	devilutionx.exe!devilution::net::protocol_zt::send_oob(const devilution::net::protocol_zt::endpoint & peer, const std::vector<unsigned char,std::allocator<unsigned char>> & data) Line 106	C++
 	devilutionx.exe!devilution::net::base_protocol<devilution::net::protocol_zt>::recv_ingame(devilution::net::packet & pkt, devilution::net::protocol_zt::endpoint sender) Line 254	C++
 	devilutionx.exe!devilution::net::base_protocol<devilution::net::protocol_zt>::recv_decrypted(devilution::net::packet & pkt, devilution::net::protocol_zt::endpoint sender) Line 238	C++
 	devilutionx.exe!devilution::net::base_protocol<devilution::net::protocol_zt>::recv() Line 180	C++
 	devilutionx.exe!devilution::net::base_protocol<devilution::net::protocol_zt>::poll() Line 150	C++
 	devilutionx.exe!devilution::net::base::SNetReceiveMessage(int * sender, void * * data, unsigned int * size) Line 117	C++
 	devilutionx.exe!devilution::net::cdwrap<devilution::net::base_protocol<devilution::net::protocol_zt>>::SNetReceiveMessage(int * sender, void * * data, unsigned int * size) Line 101	C++
 	devilutionx.exe!devilution::SNetReceiveMessage(int * senderplayerid, void * * data, unsigned int * databytes) Line 46	C++
 	devilutionx.exe!devilution::multi_process_network_packets() Line 582	C++
 	devilutionx.exe!devilution::`anonymous namespace'::RunGameLoop(devilution::interface_mode uMsg) Line 754	C++
 	devilutionx.exe!devilution::StartGame(bool bNewGame, bool bSinglePlayer) Line 1602	C++
 	devilutionx.exe!devilution::`anonymous namespace'::InitMenu(devilution::_selhero_selections type) Line 52	C++
 	devilutionx.exe!devilution::`anonymous namespace'::InitMultiPlayerMenu() Line 69	C++
 	devilutionx.exe!devilution::mainmenu_loop() Line 161	C++
 	devilutionx.exe!devilution::DiabloMain(int argc, char * * argv) Line 1640	C++
 	devilutionx.exe!SDL_main(int argc, char * * argv) Line 44	C++
 	devilutionx.exe!main_getcmdline() Line 82	C
 	devilutionx.exe!WinMain(HINSTANCE__ * hInst, HINSTANCE__ * hPrev, char * szCmdLine, int sw) Line 115	C
 	[External Code]	

StephenCWills avatar Jan 03 '22 05:01 StephenCWills