From 51f3216bee15473533963c3e9b7c37061bdb0da9 Mon Sep 17 00:00:00 2001
From: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
Date: Fri, 7 Oct 2022 17:07:46 -0700
Subject: [PATCH] bgpd: BGP fails to free the nexthop node

In case of BGP unnumbered, BGP fails to free the nexthop
node for peer if the interface is shutdown before
unconfiguring/deleting the BGP neighbor.

This is because, when the interface is shutdown,
peer's LL neighbor address will be cleared. Therefore,
during neighbor deletion, since the peer's neighbor
address is not available, BGP will skip freeing the
nexthop node of this peer. This results in a stale
nexthop node that points to a peer that's already
been freed.

Ticket: 3191547
Signed-off-by: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
---
 bgpd/bgp_nht.c | 73 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 9 deletions(-)

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index fd1aa6ab47..e387cdd488 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -212,6 +212,37 @@ void bgp_replace_nexthop_by_peer(struct peer *from, struct peer *to)
 		bnct->nht_info = to;
 }
 
+/*
+ * Returns the bnc whose bnc->nht_info matches the LL peer by
+ * looping through the IPv6 nexthop table
+ */
+static struct bgp_nexthop_cache *
+bgp_find_ipv6_nexthop_matching_peer(struct peer *peer)
+{
+	struct bgp_nexthop_cache *bnc;
+
+	frr_each (bgp_nexthop_cache, &peer->bgp->nexthop_cache_table[AFI_IP6],
+		  bnc) {
+		if (bnc->nht_info == peer) {
+			if (BGP_DEBUG(nht, NHT)) {
+				zlog_debug(
+					"Found bnc: %pFX(%u)(%u)(%p) for peer: %s(%s) %p",
+					&bnc->prefix, bnc->ifindex,
+					bnc->srte_color, bnc, peer->host,
+					peer->bgp->name_pretty, peer);
+			}
+			return bnc;
+		}
+	}
+
+	if (BGP_DEBUG(nht, NHT))
+		zlog_debug(
+			"Could not find bnc for peer %s(%s) %p in v6 nexthop table",
+			peer->host, peer->bgp->name_pretty, peer);
+
+	return NULL;
+}
+
 void bgp_unlink_nexthop_by_peer(struct peer *peer)
 {
 	struct prefix p;
@@ -219,15 +250,30 @@ void bgp_unlink_nexthop_by_peer(struct peer *peer)
 	afi_t afi = family2afi(peer->su.sa.sa_family);
 	ifindex_t ifindex = 0;
 
-	if (!sockunion2hostprefix(&peer->su, &p))
-		return;
-	/*
-	 * Gather the ifindex for if up/down events to be
-	 * tagged into this fun
-	 */
-	if (afi == AFI_IP6 && IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
-		ifindex = peer->su.sin6.sin6_scope_id;
-	bnc = bnc_find(&peer->bgp->nexthop_cache_table[afi], &p, 0, ifindex);
+	if (!sockunion2hostprefix(&peer->su, &p)) {
+		/*
+		 * In scenarios where unnumbered BGP session is brought
+		 * down by shutting down the interface before unconfiguring
+		 * the BGP neighbor, neighbor information in peer->su.sa
+		 * will be cleared when the interface is shutdown. So
+		 * during the deletion of unnumbered bgp peer, above check
+		 * will return true. Therefore, in this case,BGP needs to
+		 * find the bnc whose bnc->nht_info matches the
+		 * peer being deleted and free it.
+		 */
+		bnc = bgp_find_ipv6_nexthop_matching_peer(peer);
+	} else {
+		/*
+		 * Gather the ifindex for if up/down events to be
+		 * tagged into this fun
+		 */
+		if (afi == AFI_IP6 &&
+		    IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
+			ifindex = peer->su.sin6.sin6_scope_id;
+		bnc = bnc_find(&peer->bgp->nexthop_cache_table[afi], &p, 0,
+			       ifindex);
+	}
+
 	if (!bnc)
 		return;
 
@@ -443,6 +489,15 @@ void bgp_delete_connected_nexthop(afi_t afi, struct peer *peer)
 	if (!peer)
 		return;
 
+	/*
+	 * In case the below check evaluates true and if
+	 * the bnc has not been freed at this point, then
+	 * we might have to do something similar to what's
+	 * done in bgp_unlink_nexthop_by_peer(). Since
+	 * bgp_unlink_nexthop_by_peer() loops through the
+	 * nodes of V6 nexthop cache to find the bnc, it is
+	 * currently not being called here.
+	 */
 	if (!sockunion2hostprefix(&peer->su, &p))
 		return;
 	/*
-- 
2.39.5