connectd: handle an already-connected subd gracefully
What the developer wrote
Authored by Níckolas Goline
connectd: handle an already-connected subd gracefully
peer_connect_subd asserted that the subd had no connection yet. Replace
that assertion with defensive handling: if a connection is already
attached for this channel_id, keep it and close the incoming fd. This
avoids aborting connectd in the unexpected case where a second fd
arrives for a subd that already has one.
What changed, and why it matters
This change fixes a crash in the part of Core Lightning that handles peer connections. Previously, if the same internal connection was unexpectedly set up twice, the program would abort with an assertion failure. Now it logs a debug message and closes the duplicate connection instead. This makes the node more stable against unusual or possibly malicious connection patterns, but the patch is defensive and does not by itself prove an attacker could reliably trigger the crash.
Treat as a stability/hardening fix. Review how duplicate peer_connect_subd messages can be generated to ensure the root cause is not reachable from an untrusted peer. Consider adding a regression test that sends a second fd for the same channel_id and verifies connectd no longer aborts.
Security signals we found
Assertion replaced with defensive error handling
Potential denial-of-service crash vector removed
Duplicate peer/subd connection now logged and dropped
No input validation or cryptographic changes
Evidence from the diff
In connectd/multiplex.c, peer_connect_subd() previously asserted that subd->conn was NULL. The patch replaces that assert with a runtime check: if subd->conn is already set, it logs a debug message, closes the incoming fd, and returns. This prevents connectd from aborting when a second file descriptor arrives for a subd that already has an attached connection. The change is defensive hardening rather than a complete fix for any underlying duplicate-subd routing bug.
Changed components
connectd/multiplex.cpeer_connect_subd()subd connection state managementInspect captured patch +10 / −1
diff --git a/connectd/multiplex.c b/connectd/multiplex.c
index 1776d541..1051645f 100644
--- a/connectd/multiplex.c
+++ b/connectd/multiplex.c
@@ -1729,7 +1729,16 @@ void peer_connect_subd(struct daemon *daemon, const u8 *msg, int fd)
fmt_node_id(tmpctx, &id)));
}
- assert(!subd->conn);
+ /* We only keep one connection per channel_id. If one is already
+ * attached for this channel_id, drop this fd rather than replacing
+ * it. */
+ if (subd->conn) {
+ status_peer_debug(&id,
+ "Already have a subd for channel_id %s: ignoring",
+ fmt_channel_id(tmpctx, &channel_id));
+ close(fd);
+ return;
+ }
/* This sets subd->conn inside subd_conn_init, and reparents subd! */
io_new_conn(peer, fd, subd_conn_init, subd);
Why this scored 42/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.