diff --git a/hostmap.go b/hostmap.go index 2f2db101..b9acdd62 100644 --- a/hostmap.go +++ b/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 diff --git a/relay_manager.go b/relay_manager.go index 318a9f1a..1ae382a3 100644 --- a/relay_manager.go +++ b/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{