Zombie connection after lightningd ignores `connectd_peer_spoke`

[morehouse]

When connectd reads a peer message whose channel_id has no attached subd, it creates a subd with conn == NULL and sends connectd_peer_spoke to lightningd, asking it to start up a subdaemon and respond with connectd_peer_connect_subd and the file descriptor that should be assigned to conn.

While connectd is waiting for lightningd's response, it stops reading all messages from the peer:

lightning/connectd/multiplex.c

Lines 1506 to 1538 in ae53e87

	status_peer_debug(&peer->id, "Activating for message %s",
	peer_wire_name(t));
	subd = new_subd(peer, &channel_id);
	/* We tell lightningd to fire up a subdaemon to handle this! */
	daemon_conn_send(peer->daemon->master,
	take(towire_connectd_peer_spoke(NULL, &peer->id,
	peer->counter,
	t,
	&channel_id,
	is_peer_error(tmpctx, decrypted))));
	}
	/* Even if we just created it, call this to catch open_channel2 */
	maybe_update_channelid(subd, decrypted);
	/* Tell them to write. */
	msg_enqueue(subd->outq, take(decrypted));
	/* Is this a tx_abort? Ignore from now on, and close after sending! */
	if (type == WIRE_TX_ABORT) {
	subd->rcvd_tx_abort = true;
	/* In case it doesn't close by itself */
	notleak(new_reltimer(&peer->daemon->timers, subd,
	time_from_sec(5),
	close_subd_timeout, subd));
	}
	/* Wait for them to wake us */
	peer->peer_in_lastmsg = type;
	out:
	peer->peer_in_lasttime = time_mono();
	return io_wait(peer_conn, &peer->peer_in, next_read, peer);

The io_wait at the end is only released when either lightningd responds, or when CLN needs to send a message to that peer (e.g., gossip flush). If neither happens, the connection becomes a zombie and all peer messages are ignored until eventually the ping timeout expires and the connection is dropped.

There are currently six situations where lightningd's handle_peer_spoke never responds:

1. The peer sends an error for its first message (e.g., data loss recovery).
   

   lightning/lightningd/peer_control.c

   Lines 2067 to 2072 in ae53e87

   	/* If they send an error, handle it immediately. */
   	if (errmsg) {
   	channel_fail_permanent(channel, REASON_REMOTE,
   	"They sent %s", errmsg);
   	return;
   	}

2. channeld dies from a bug or protocol violation, and the peer sends another message for that channel before lightningd recognizes the channeld died.
   

   lightning/lightningd/peer_control.c

   Lines 2074 to 2081 in ae53e87

   	/* If channel is active there are two possibilities:
   	* 1. We have started subd, but channeld hasn't processed
   	* the connectd_peer_connect_subd message yet.
   	* 2. subd exited */
   	if (channel->owner) {
   	/* We raced... */
   	return;
   	}

3. The peer attempts a channel_reestablish while the node is shutting down.
   

   lightning/lightningd/peer_control.c

   Lines 2083 to 2097 in ae53e87

   	if (msgtype == WIRE_CHANNEL_REESTABLISH
   	&& ignore_idle_channel(ld, channel)) {
   	log_debug(channel->log,
   	"Peer sent channel_reestablish, but gracefully shutting down; "
   	"sending warning and ignoring");
   	error = towire_warningfmt(tmpctx, &channel_id,
   	"Declining to reestablish idle channel "
   	"because this node will be halting soon.");
   	/* Don't goto send_error; we don't want to disconnect. */
   	subd_send_msg(ld->connectd,
   	take(towire_connectd_peer_send_msg(NULL, &peer->id,
   	peer->connectd_counter,
   	error)));
   	return;
   	}

4. The peer sends a message after channeld was killed without a status message (e.g., OOM).
   

   lightning/lightningd/peer_control.c

   Lines 2100 to 2109 in ae53e87

   	if (channel->state == DUALOPEND_AWAITING_LOCKIN) {
   	pfd = sockpair(tmpctx, channel, &other_fd, &error);
   	if (!pfd)
   	goto send_error;
   	if (peer_restart_dualopend(peer, pfd, channel, false))
   	goto tell_connectd;
   	/* FIXME: Send informative error? */
   	close(other_fd);
   	}
   	return;

5. openingd fails to spawn (e.g., fd limit reached).
   

   lightning/lightningd/peer_control.c

   Lines 2143 to 2145 in ae53e87

   	/* FIXME: Send informative error? */
   	close(other_fd);
   	return;

6. dualopend fails to spawn (e.g., fd limit reached).
   

   lightning/lightningd/peer_control.c

   Lines 2163 to 2165 in ae53e87

   	/* FIXME: Send informative error? */
   	close(other_fd);
   	return;

Suggested fix

Add a new message connectd_peer_no_subd that lightningd can respond with when handle_peer_spoke fails in the above situations. Then connectd knows to free the matching subd and continue reading from the connection.

Discovery

This bug was discovered while fuzzing CLN with smite. Smite would send a channel_ready message with an incorrect channel_id, causing channeld to exit. Then smite would send another channel message, which would trigger the Situation 2 race condition and zombify the connection.