lightningd: fix race with mutual connect.
What the developer wrote
Authored by Rusty Russell
lightningd: fix race with mutual connect.
65dccea5bde4 "pytest: fix flake in test_reconnect_signed" accidentally
introduced a bug, where the connect command may not return.
If we call "connect" while a connection is still being processed
through the peer_connected hooks, we would call peer_channels_cleanup(),
which (if the peer has no channels) would free the peer.
Then when the peer_connected hook returned, it would lookup the peer,
see it was gone, and silently return. The connect_succeeded() function
was never called, and the connect command never woken.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None: bug introduced this release.
What changed, and why it matters
This is a bug fix for a race condition in Core Lightning's peer connection handling. If a user or automated process called the 'connect' command while a peer connection was still being finalized through internal hooks, the connection command could hang forever and never return a response. The fix ensures the command waits properly during the 'connecting' state instead of cleaning up the peer prematurely. There is no direct evidence this was exploited as a security vulnerability; it appears to be a reliability bug.
Apply the patch. Monitor for any related hangs in connect command responses. No immediate incident response is indicated unless RPC unavailability is observed.
Security signals we found
Race condition in connection state machine
Use-after-free-like pattern: peer freed while hooks still reference it
Command hang / denial of service via unresponsive RPC call
Regression introduced by prior commit
Evidence from the diff
The commit fixes a regression introduced by 65dccea5bde4. Previously, json_connect() would call peer_channels_cleanup() on any known peer that was not already PEER_CONNECTED. If the peer was in PEER_CONNECTING state, this cleanup could free the peer before peer_connected hooks completed. When the hook later ran, it would find the peer missing and silently return without calling connect_succeeded(), leaving the connect command unawakened. The patch restructures the state handling: PEER_CONNECTED returns success immediately, PEER_DISCONNECTED forces cleanup, and PEER_CONNECTING simply waits for the in-progress connection to complete.
Changed components
lightningd/connect_control.cjson_connect RPC commandpeer connection lifecycle managementInspect captured patch +19 / −12
diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c
index c2a28522..e57bc975 100644
--- a/lightningd/connect_control.c
+++ b/lightningd/connect_control.c
@@ -219,21 +219,28 @@ static struct command_result *json_connect(struct command *cmd,
/* If we know about peer, see if it's already connected. */
peer = peer_by_id(cmd->ld, &id_addr.id);
- if (peer && peer->connected == PEER_CONNECTED) {
- log_debug(cmd->ld->log, "Already connected via %s",
- fmt_wireaddr_internal(tmpctx,
+ if (peer) {
+ switch (peer->connected) {
+ case PEER_CONNECTED:
+ log_debug(cmd->ld->log, "Already connected via %s",
+ fmt_wireaddr_internal(tmpctx,
&peer->addr));
- return connect_cmd_succeed(cmd, peer,
- peer->connected_incoming,
- &peer->addr);
+ return connect_cmd_succeed(cmd, peer,
+ peer->connected_incoming,
+ &peer->addr);
+ case PEER_DISCONNECTED:
+ /* When a peer disconnects, we give subds time to clean themselves up
+ * (this lets connectd ensure they've seen the final messages). But
+ * now it's going to try to reconnect, we've gotta force them out. */
+ peer_channels_cleanup(peer);
+ break;
+ case PEER_CONNECTING:
+ /* Just wait until connection finished. Though we still ask connectd to connect,
+ * it's going to ignore it. */
+ break;
+ }
}
- /* When a peer disconnects, we give subds time to clean themselves up
- * (this lets connectd ensure they've seen the final messages). But
- * now it's going to try to reconnect, we've gotta force them out. */
- if (peer)
- peer_channels_cleanup(peer);
-
subd_send_msg(cmd->ld->connectd,
take(towire_connectd_connect_to_peer(NULL, &id_addr.id,
addr, true,
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.