Merge pull request #14886 from LabNConsulting/aceelindem/ospf-link-local-fix

ospfd: Fix OSPF link-local opaque LSA crash and opaque memory corruption

5 files changed, 58 insertions, 25 deletions

diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c
index 12337abcb3..0969ae15bd 100644
--- a/ospfd/ospf_interface.c
+++ b/ospfd/ospf_interface.c
@@ -339,6 +339,8 @@ void ospf_if_free(struct ospf_interface *oi)
 
 	assert(oi->state == ISM_Down);
 
+	ospf_opaque_type9_lsa_if_cleanup(oi);
+
 	ospf_opaque_type9_lsa_term(oi);
 
 	QOBJ_UNREG(oi);
@@ -380,26 +382,6 @@ int ospf_if_is_up(struct ospf_interface *oi)
 	return if_is_up(oi->ifp);
 }
 
-struct ospf_interface *ospf_if_exists(struct ospf_interface *oic)
-{
-	struct listnode *node;
-	struct ospf *ospf;
-	struct ospf_interface *oi;
-
-	if (!oic)
-		return NULL;
-
-	ospf = oic->ospf;
-	if (ospf == NULL)
-		return NULL;
-
-	for (ALL_LIST_ELEMENTS_RO(ospf->oiflist, node, oi))
-		if (oi == oic)
-			return oi;
-
-	return NULL;
-}
-
 /* Lookup OSPF interface by router LSA posistion */
 struct ospf_interface *ospf_if_lookup_by_lsa_pos(struct ospf_area *area,
 						 int lsa_pos)
diff --git a/ospfd/ospf_interface.h b/ospfd/ospf_interface.h
index e2290a881c..08a2b11273 100644
--- a/ospfd/ospf_interface.h
+++ b/ospfd/ospf_interface.h
@@ -300,7 +300,6 @@ extern int ospf_if_up(struct ospf_interface *oi);
 extern int ospf_if_down(struct ospf_interface *oi);
 
 extern int ospf_if_is_up(struct ospf_interface *oi);
-extern struct ospf_interface *ospf_if_exists(struct ospf_interface *oi);
 extern struct ospf_interface *ospf_if_lookup_by_lsa_pos(struct ospf_area *area,
 							int lsa_pos);
 extern struct ospf_interface *
diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c
index 7c3f289e02..08aa103686 100644
--- a/ospfd/ospf_nsm.c
+++ b/ospfd/ospf_nsm.c
@@ -219,7 +219,7 @@ static int ospf_db_summary_add(struct ospf_neighbor *nbr, struct ospf_lsa *lsa)
 	case OSPF_OPAQUE_LINK_LSA:
 		/* Exclude type-9 LSAs that does not have the same "oi" with
 		 * "nbr". */
-		if (ospf_if_exists(lsa->oi) != nbr->oi)
+		if (lsa->oi != nbr->oi)
 			return 0;
 		break;
 	case OSPF_OPAQUE_AREA_LSA:
diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index 27f47a6d79..86492c7b13 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -427,10 +427,12 @@ void ospf_delete_opaque_functab(uint8_t lsa_type, uint8_t opaque_type)
 				if (functab->oipt != NULL)
 					free_opaque_info_per_type(functab->oipt,
 								  true);
-				/* Dequeue listnode entry from the list. */
+				/* Dequeue listnode entry from the function table
+				 * list coreesponding to the opaque LSA type.
+				 * Note that the list deletion callback frees
+				 * the functab entry memory.
+				 */
 				listnode_delete(funclist, functab);
-
-				XFREE(MTYPE_OSPF_OPAQUE_FUNCTAB, functab);
 				break;
 			}
 		}
@@ -614,6 +616,17 @@ static void free_opaque_info_per_type(struct opaque_info_per_type *oipt,
 		}
 		listnode_delete(l, oipt);
 	}
+
+	/*
+	 * Delete the function table corresponding to the LSA type and opaque type
+	 * as well. The pointer to the opaque per-type information structure in
+	 * the function table structure be set to NULL to avoid recursion during
+	 * deletion.
+	 */
+	if (oipt->functab) {
+		oipt->functab->oipt = NULL;
+		ospf_delete_opaque_functab(oipt->lsa_type, oipt->opaque_type);
+	}
 	XFREE(MTYPE_OPAQUE_INFO_PER_TYPE, oipt);
 	return;
 }
@@ -746,6 +759,44 @@ int ospf_opaque_is_owned(struct ospf_lsa *lsa)
 	return (oipt != NULL && lookup_opaque_info_by_id(oipt, lsa) != NULL);
 }
 
+/*
+ * Cleanup Link-Local LSAs assocaited with an interface that is being deleted.
+ * Since these LSAs are stored in the area link state database (LSDB) as opposed
+ * to a separate per-interface, they must be deleted from the area database.
+ * Since their flooding scope is solely the deleted OSPF interface, there is no
+ * need to attempt to flush them from the routing domain. For link local LSAs
+ * originated via the OSPF server API, LSA deletion before interface deletion
+ * is required so that the callback can access the OSPF interface address.
+ */
+void ospf_opaque_type9_lsa_if_cleanup(struct ospf_interface *oi)
+{
+	struct route_node *rn;
+	struct ospf_lsdb *lsdb;
+	struct ospf_lsa *lsa;
+
+	lsdb = oi->area->lsdb;
+	LSDB_LOOP (OPAQUE_LINK_LSDB(oi->area), rn, lsa)
+		/*
+		 * While the LSA shouldn't be referenced on any LSA
+		 * lists since the flooding scoped is confined to the
+		 * interface being deleted, clear the pointer to the
+		 * deleted interface for safety's sake after it is
+		 * removed from the area LSDB.
+		 */
+		if (lsa->oi == oi) {
+			if (IS_DEBUG_OSPF_EVENT)
+				zlog_debug("Delete Type-9 Opaque-LSA on interface delete: [opaque-type=%u, opaque-id=%x]",
+					   GET_OPAQUE_TYPE(
+						   ntohl(lsa->data->id.s_addr)),
+					   GET_OPAQUE_ID(ntohl(
+						   lsa->data->id.s_addr)));
+			ospf_lsa_lock(lsa);
+			ospf_lsdb_delete(lsdb, lsa);
+			lsa->oi = NULL;
+			ospf_lsa_unlock(&lsa);
+		}
+}
+
 /*------------------------------------------------------------------------*
  * Following are (vty) configuration functions for Opaque-LSAs handling.
  *------------------------------------------------------------------------*/
diff --git a/ospfd/ospf_opaque.h b/ospfd/ospf_opaque.h
index e0c78cd6dc..54651f8f58 100644
--- a/ospfd/ospf_opaque.h
+++ b/ospfd/ospf_opaque.h
@@ -109,6 +109,7 @@ extern void ospf_opaque_term(void);
 extern void ospf_opaque_finish(void);
 extern int ospf_opaque_type9_lsa_init(struct ospf_interface *oi);
 extern void ospf_opaque_type9_lsa_term(struct ospf_interface *oi);
+extern void ospf_opaque_type9_lsa_if_cleanup(struct ospf_interface *oi);
 extern int ospf_opaque_type10_lsa_init(struct ospf_area *area);
 extern void ospf_opaque_type10_lsa_term(struct ospf_area *area);
 extern int ospf_opaque_type11_lsa_init(struct ospf *ospf);