don't make new relay state on a just-discarded tunnel (#1796)

Author: JackDoan

hostmap.go

@@ -448,13 +448,15 @@ func (hm *HostMap) MakePrimary(hostinfo *HostInfo) {
 	hm.unlockedMakePrimary(hostinfo)
 }
 
-func (hm *HostMap) unlockedMakePrimary(hostinfo *HostInfo) {
+// unlockedMakePrimary reports whether hostinfo is (now) the primary for each of its addresses,
+// false only when it is no longer in the hostmap at all.
+func (hm *HostMap) unlockedMakePrimary(hostinfo *HostInfo) bool {
 	// A hostinfo that is no longer in the hostmap must not be re-inserted here. Callers can race
 	// tunnel teardown, deciding to promote under the read lock and only taking the write lock
 	// after a delete fully unlinked the hostinfo (connection manager swapPrimary, AddRelay). Every
 	// live hostinfo is registered in Indexes by unlockedAddHostInfo, so this is a membership test.
 	if hm.Indexes[hostinfo.localIndexId] != hostinfo {
-		return
+		return false
 	}
 
 	// Move hostinfo to the front (primary) of each of its address lists. The lists are
@@ -469,6 +471,7 @@ func (hm *HostMap) unlockedMakePrimary(hostinfo *HostInfo) {
 		list = append([]*HostInfo{hostinfo}, list...)
 		hm.unlockedSetHostsForAddr(addr, list)
 	}
+	return true
 }
 
 // unlockedDeleteHostInfo removes hostinfo from every one of its address lists and from the index

relay_manager.go

@@ -107,7 +107,10 @@ func (rm *relayManager) StartRelays(f *Interface, vpnIp netip.Addr, hh *Handshak
 			if relayHostInfo.GetRemote().IsValid() {
 				idx, err := AddRelay(rm.l, relayHostInfo, rm.hostmap, vpnIp, nil, TerminalType, Requested)
 				if err != nil {
+					// No local relay state was installed, so a CreateRelayRequest would hand the
+					// peer an index we could never resolve. Skip it.
 					hl.Info("Failed to add relay to hostmap", "relay", relay.String(), "error", err)
+					continue
 				}
 
 				m := NebulaControl{
@@ -237,7 +240,12 @@ func AddRelay(l *slog.Logger, relayHostInfo *HostInfo, hm *HostMap, vpnIp netip.
 			// Avoid standing up a relay that can't be used since only the primary hostinfo
 			// will be pointed to by the relay logic
 			//TODO: if there was an existing primary and it had relay state, should we merge?
-			hm.unlockedMakePrimary(relayHostInfo)
+			if !hm.unlockedMakePrimary(relayHostInfo) {
+				// The tunnel was torn down after the caller grabbed relayHostInfo. A relay standing
+				// on an unlinked hostinfo would never carry traffic, and its Relays entry could
+				// never be reclaimed since the delete-time cleanup has already run.
+				return 0, errors.New("relay hostinfo is no longer in the hostmap")
+			}
 
 			hm.Relays[index] = relayHostInfo
 			newRelay := Relay{