From fd90e5746bf1a669fdedb79071c0a33556b31cef Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Sun, 19 Jun 2022 16:01:42 +0930
Subject: [PATCH] connectd: don't keep around more than one old connection.

This was fixed in 1c495ca5a83df8cccb62a53cbfd3bc437ca3745c ("connectd:
fix accidental handling of old reconnections.") and then reverted by
the rework in "connectd: avoid use-after-free upon multiple
reconnections by a peer".

The latter made the race much less likely, since we cleaned up the
reconnecting struct once the connection was hung up by the remote
node, but it's still theoretically possible.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 connectd/connectd.c              | 17 ++++++++++++++---
 connectd/connectd.h              |  3 ++-
 connectd/peer_exchange_initmsg.c |  3 ++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/connectd/connectd.c b/connectd/connectd.c
index 9a5d7f211..07a952732 100644
--- a/connectd/connectd.c
+++ b/connectd/connectd.c
@@ -239,7 +239,8 @@ static struct io_plan *retry_peer_connected(struct io_conn *conn,
 	/*~ Usually the pattern is to return this directly. */
 	return peer_connected(conn, pr->daemon, &pr->id, &pr->addr,
 			      pr->remote_addr,
-			      &pr->cs, take(pr->their_features), pr->incoming);
+			      &pr->cs, take(pr->their_features), pr->incoming,
+			      true);
 }
 
 /*~ If we already know about this peer, we tell lightningd and it disconnects
@@ -334,7 +335,8 @@ struct io_plan *peer_connected(struct io_conn *conn,
 			       const struct wireaddr *remote_addr,
 			       struct crypto_state *cs,
 			       const u8 *their_features TAKES,
-			       bool incoming)
+			       bool incoming,
+			       bool retrying)
 {
 	u8 *msg;
 	struct peer *peer;
@@ -344,9 +346,18 @@ struct io_plan *peer_connected(struct io_conn *conn,
 	bool option_gossip_queries;
 
 	peer = peer_htable_get(&daemon->peers, id);
-	if (peer)
+	if (peer) {
+		/* If we were already retrying, we only get one chance: there
+		 * can be multiple reconnections, and we must not keep around
+		 * stale ones */
+		if (retrying) {
+			if (taken(their_features))
+				tal_free(their_features);
+			return io_close(conn);
+		}
 		return peer_reconnected(conn, daemon, id, addr, remote_addr, cs,
 					their_features, incoming);
+	}
 
 	/* We promised we'd take it by marking it TAKEN above; prepare to free it. */
 	if (taken(their_features))
diff --git a/connectd/connectd.h b/connectd/connectd.h
index 02fe5bf23..9af5f815d 100644
--- a/connectd/connectd.h
+++ b/connectd/connectd.h
@@ -212,7 +212,8 @@ struct io_plan *peer_connected(struct io_conn *conn,
 			       const struct wireaddr *remote_addr,
 			       struct crypto_state *cs,
 			       const u8 *their_features TAKES,
-			       bool incoming);
+			       bool incoming,
+			       bool retrying);
 
 /* Called when peer->peer_conn is finally freed */
 void peer_conn_closed(struct peer *peer);
diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c
index fa9799838..c9009a007 100644
--- a/connectd/peer_exchange_initmsg.c
+++ b/connectd/peer_exchange_initmsg.c
@@ -137,7 +137,8 @@ static struct io_plan *peer_init_received(struct io_conn *conn,
 			      remote_addr,
 			      &peer->cs,
 			      take(features),
-			      peer->incoming);
+			      peer->incoming,
+			      false);
 }
 
 static struct io_plan *peer_init_hdr_received(struct io_conn *conn,