Race conditions in VirtualTap::put()
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]