ospf6d: Fix ABR brouter calculation corruption

During Intra brouter calculation, brouters will be
marked for remove. if one of the brouter is removed,
as part of its remove callback, ospf6_abr_examin_summary
is performed where marked for brouter would be removed.
Since refcount of next brouter node still higher, it will
retain one node with dangled next brouter pointer.
When intra brouter calculation iteration goes to next node,
where accessing free node causes a crash.

Ticket:CM-20807
Testing Done:
Configure multilple ABR routers between area 0 and area x, y.
Remove ospf6 configuration on area x, y abrs and check area 0
Intra brouter calculations.

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>

ospf6d/ospf6_abr.c

@@ -770,7 +770,8 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 	}
 	if (OSPF6_LSA_IS_MAXAGE(lsa)) {
 		if (is_debug)
-			zlog_debug("LSA is MaxAge, ignore");
+			zlog_debug("%s: LSA %s is MaxAge, ignore",
+				   __PRETTY_FUNCTION__, lsa->name);
 		if (old)
 			ospf6_route_remove(old, table);
 		return;
@@ -847,9 +848,24 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 	    || CHECK_FLAG(abr_entry->flag, OSPF6_ROUTE_REMOVE)
 	    || !CHECK_FLAG(abr_entry->path.router_bits, OSPF6_ROUTER_BIT_B)) {
 		if (is_debug)
-			zlog_debug("ABR router entry does not exist, ignore");
+			zlog_debug("%s: ABR router entry does not exist, ignore",
-		if (old)
+				   __PRETTY_FUNCTION__);
-			ospf6_route_remove(old, table);
+		if (old) {
+			if (old->type == OSPF6_DEST_TYPE_ROUTER &&
+			    oa->intra_brouter_calc) {
+				if (is_debug)
+					zlog_debug(
+						   "%s: intra_brouter_calc is on, skip brouter remove: %s (%p)",
+						   __PRETTY_FUNCTION__, buf,
+						   (void *)old);
+			} else {
+				if (is_debug)
+					zlog_debug("%s: remove old entry: %s %p ",
+						   __PRETTY_FUNCTION__, buf,
+						   (void *)old);
+				ospf6_route_remove(old, table);
+			}
+		}
 		return;
 	}
 

ospf6d/ospf6_area.h

@@ -50,6 +50,9 @@ struct ospf6_area {
 	/* Area type */
 	int no_summary;
 
+	/* Brouter traversal protection */
+	int intra_brouter_calc;
+
 	/* OSPF interface list */
 	struct list *if_list;
 

ospf6d/ospf6_intra.c

@@ -2047,8 +2047,10 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 	uint32_t brouter_id;
 	char brouter_name[16];
 
-	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id))
+	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id) ||
-		zlog_info("border-router calculation for area %s", oa->name);
+	    IS_OSPF6_DEBUG_ROUTE(MEMORY))
+		zlog_info("%s: border-router calculation for area %s",
+			  __PRETTY_FUNCTION__, oa->name);
 
 	hook_add = oa->ospf6->brouter_table->hook_add;
 	hook_remove = oa->ospf6->brouter_table->hook_remove;
@@ -2114,6 +2116,7 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 
 	for (brouter = ospf6_route_head(oa->ospf6->brouter_table); brouter;
 	     brouter = nbrouter) {
+
 		/*
 		 * brouter may have been "deleted" in the last loop iteration.
 		 * If this is the case there is still 1 final refcount lock
@@ -2122,6 +2125,8 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 		 * skip processing the deleted route.
 		 */
 		if (brouter->lock == 1) {
+			if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
+				ospf6_brouter_debug_print(brouter);
 			nbrouter = ospf6_route_next(brouter);
 			continue;
 		} else {
@@ -2173,8 +2178,14 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 				       brouter_id)
 			    || IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(
 				       oa->area_id))
-				zlog_info("brouter %s disappears via area %s",
+				zlog_info("%s: brouter %s disappears via area %s",
-					  brouter_name, oa->name);
+					  __PRETTY_FUNCTION__, brouter_name,
+					  oa->name);
+			/* This is used to protect nbrouter from removed from
+			 * the table. For an example, ospf6_abr_examin_summary,
+			 * removes brouters which are marked for remove.
+			 */
+			oa->intra_brouter_calc = 1;
 			ospf6_route_remove(brouter, oa->ospf6->brouter_table);
 			brouter = NULL;
 		} else if (CHECK_FLAG(brouter->flag, OSPF6_ROUTE_ADD)
@@ -2184,8 +2195,9 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 				       brouter_id)
 			    || IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(
 				       oa->area_id))
-				zlog_info("brouter %s appears via area %s",
+				zlog_info("%s: brouter %s appears via area %s",
-					  brouter_name, oa->name);
+					  __PRETTY_FUNCTION__, brouter_name,
+					  oa->name);
 
 			/* newly added */
 			if (hook_add)
@@ -2205,11 +2217,14 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 			UNSET_FLAG(brouter->flag, OSPF6_ROUTE_ADD);
 			UNSET_FLAG(brouter->flag, OSPF6_ROUTE_CHANGE);
 		}
+		/* Reset for nbrouter */
+		oa->intra_brouter_calc = 0;
 	}
 
-	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id))
+	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id) ||
-		zlog_info("border-router calculation for area %s: done",
+	    IS_OSPF6_DEBUG_ROUTE(MEMORY))
-			  oa->name);
+		zlog_info("%s: border-router calculation for area %s: done",
+			  __PRETTY_FUNCTION__, oa->name);
 }
 
 struct ospf6_lsa_handler router_handler = {.lh_type = OSPF6_LSTYPE_ROUTER,

ospf6d/ospf6_route.c

@@ -925,10 +925,11 @@ struct ospf6_route *ospf6_route_next(struct ospf6_route *route)
 	struct ospf6_route *next = route->next;
 
 	if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
-		zlog_info("%s %p: route next: %p<-[%p]->%p",
+		zlog_info("%s %p: route next: %p<-[%p]->%p , route ref count %u",
 			  ospf6_route_table_name(route->table),
 			  (void *)route->table, (void *)route->prev,
-			  (void *)route, (void *)route->next);
+			  (void *)route, (void *)route->next,
+			  route->lock);
 
 	ospf6_route_unlock(route);
 	if (next)