Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeService heap corruption after ZTS_EVENT_PEER_PATH_DISCOVERED #261

Closed
jbatnozic opened this issue Nov 16, 2023 · 6 comments
Closed

NodeService heap corruption after ZTS_EVENT_PEER_PATH_DISCOVERED #261

jbatnozic opened this issue Nov 16, 2023 · 6 comments

Comments

@jbatnozic
Copy link

Hello,

I've started seeing libzt produce heap corruption errors when nodes try to connect. Sometimes - I wasn't able to determine a clear pattern, though it consistently happens when I try to connect a 3rd node from a different computer.

I gathered screenshots from the debugger showing which part of code breaks (it's the same every time).
libzt_error_0
libzt_error_1
libzt_error_2
libzt_error_4

I also have some logs from the executable that crashed (it's a mix of ZeroTier and other logs but all lines coming from ZeroTier contain "hobrobot::ZeroTier").

[2023-11-16 00:03:19.452393] [ INFO] hobrobot::ZeroTier: ZeroTier is enabled: Configuring service...
[2023-11-16 00:03:19.455862] [ INFO] hobrobot::ZeroTier: Starting service...
[2023-11-16 00:03:19.471707] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:21.544881] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:21.575096] [ WARN] hobrobot::ZeroTier: Unknown event dispatched!
[2023-11-16 00:03:21.575580] [ INFO] hobrobot::ZeroTier: Node event: Up (ZTS_EVENT_NODE_UP)
[2023-11-16 00:03:21.576354] [ INFO] hobrobot::ZeroTier: Address event: AddedIPv4 (ZTS_EVENT_ADDR_ADDED_IP4)
    IP address: 172.26.173.137
    Node ID: d3ecf5726d81ccb3
    Comment: This is the node's virtual IP address on the specified network.
[2023-11-16 00:03:22.062375] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:22.234597] [ INFO] hobrobot::ZeroTier: Network event: Update (ZTS_EVENT_NETWORK_UPDATE)
    Network ID: d3ecf5726d81ccb3
    Comment: Network state has changed.
[2023-11-16 00:03:22.577270] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:23.089073] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:23.604506] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:24.120319] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:24.634310] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:25.139443] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:25.654410] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:26.155719] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:26.659670] [ INFO] hobrobot::ZeroTier: Waiting for local node to come online...
[2023-11-16 00:03:26.921512] [ INFO] hobrobot::ZeroTier: Node event: Online (ZTS_EVENT_NODE_ONLINE)
    Node ID: 23233a43da
[2023-11-16 00:03:26.952730] [ INFO] hobrobot::ZeroTier: Network event: ReadyIPv4 (ZTS_EVENT_NETWORK_READY_IP4)
    Network ID: d3ecf5726d81ccb3
    Comment: Configuration received. IPv4 traffic can now be sent over the network.
[2023-11-16 00:03:26.954816] [ INFO] hobrobot::ZeroTier: Network event: OK (ZTS_EVENT_NETWORK_OK)
    Network ID: d3ecf5726d81ccb3
[2023-11-16 00:03:26.956896] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=62f865ae71, Role=Planet (Root server / P2P connection orchestrator)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:26.959186] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=778cde7190, Role=Planet (Root server / P2P connection orchestrator)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:26.961883] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=cafe04eba9, Role=Planet (Root server / P2P connection orchestrator)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:26.963900] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=cafe9efeb9, Role=Planet (Root server / P2P connection orchestrator)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:26.965843] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=d3ecf5726d, Role=Leaf (Normal node)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:27.122550] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=62f865ae71, Role=Planet (Root server / P2P connection orchestrator)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:27.170450] [ INFO] hobrobot::ZeroTier: Joining network 0xd3ecf5726d81ccb3
[2023-11-16 00:03:27.171435] [ INFO] hobrobot::ZeroTier: Don't forget to authorize this device in my.zerotier.com or the web API!
[2023-11-16 00:03:27.175140] [ INFO] hobrobot::ZeroTier:
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
* The app has been cussessfully configured to communicate via ZeroTier!
* IP address of the local node on the ZT network is displayed below:
* 172.26.173.137
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

Server started on port 1 for up to 6 clients.
Initializing cpSpace - Chipmunk v7.0.3 (Debug Enabled)
Compile with -DNDEBUG defined to disable debug mode and runtime assertion checks
[2023-11-16 00:03:36.689756] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:36.690667] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:36.691309] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:36.691771] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:36.692140] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:36.692513] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=d1b50a91cb, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:36.706046] [ INFO] jbatnozic::spempe::DefaultNetworkingManager: Received event: New client (0) connected.
[2023-11-16 00:03:36.708190] [ INFO] hobrobot::EnvironmentManager: Composing SetTerrain message for client 0.
[2023-11-16 00:03:36.721237] [ INFO] jbatnozic::spempe::DefaultLobbyBackendManager: Inserted new player into slot 1
[2023-11-16 00:03:36.721552] [ INFO] LobbyFrontendManager: (event) Lobby changed.
[2023-11-16 00:03:36.741557] [ WARN] jbatnozic::spempe::DefaultLobbyBackendManager: USPEMPE_DefaultLobbyBackendManager_SetPlayerInfo_Impl - Could not find client with corresponding idx 0 amongst locked in clients.
[2023-11-16 00:03:36.743529] [ INFO] jbatnozic::spempe::DefaultAuthorizationManager: Authorized player Jovan (172.26.176.240).
[2023-11-16 00:03:55.449645] [ INFO] hobrobot::ZeroTier: Peer event: Relay (ZTS_EVENT_PEER_RELAY)
    Node info: ID=e0185f5af6, Role=Leaf (Normal node)
    Comment: No direct path is known to this node.
[2023-11-16 00:03:55.540689] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=e0185f5af6, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.
[2023-11-16 00:03:55.541323] [ INFO] hobrobot::ZeroTier: Peer event: Direct (ZTS_EVENT_PEER_DIRECT)
    Node info: ID=e0185f5af6, Role=Leaf (Normal node)
    Comment: A direct path is known to this node.
[2023-11-16 00:03:55.543206] [ INFO] hobrobot::ZeroTier: Peer event: PathDiscovered (ZTS_EVENT_PEER_PATH_DISCOVERED)
    Node info: ID=e0185f5af6, Role=Leaf (Normal node)
    Comment: A new direct path to this node was discovered.

Unfortunately I do not have a minimal reproducible example as it took a pretty massive stack of tech to get here, but I'm hoping someone will look at the code and find some obvious flaw...

Libzt I am using is compiled from commit 8d21a265cc23dd6e6e4d2c2ad068e978f110f8e3.

Everything compiled with MSVC.

@jbatnozic jbatnozic changed the title NodeService heap corruption NodeService heap corruption after ZTS_EVENT_PEER_PATH_DISCOVERED Nov 16, 2023
@jbatnozic
Copy link
Author

It looks to me like the problem is here:

zts_peer_info_t* pd = new zts_peer_info_t();
ZT_Peer* peer = (ZT_Peer*)obj;
memcpy(pd, peer, sizeof(zts_peer_info_t));

zts_peer_info_t and ZT_Peer are not the same type (different binary layouts) but this code treats them as such.

@janvanbouwel
Copy link
Contributor

The problem is not different binary layouts as the for-loop fixes that. This is the same issue as I had in #208 which was fixed in #209, so updating should solve that issue (or manually pulling in that change).

I did notice that zts_peer_info_t->path_count and ZT_Peer->pathCount are not in the same place so that information should be copied too.

@jbatnozic
Copy link
Author

I managed to build the latest version of libzt and so far I haven't seen the error reproduced, though I'll need to test it more when I have the time.

I did notice that zts_peer_info_t->path_count and ZT_Peer->pathCount are not in the same place so that information should be copied too.

Is this something that should be taken care of?

@janvanbouwel
Copy link
Contributor

Nice, glad it works!

Shouldn't matter for your issue but yes. I was ready with a PR but noticed more uninitialised memory in the for-loop so it'll take a bit longer.

@jbatnozic
Copy link
Author

So you got that covered, awesome. I will test this a bit more in the evening and if it doesn't reproduce I will close the issue.

@jbatnozic
Copy link
Author

Issue not observed on the latest version of libzt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants