ospfd: fix memory leak on SPF calculation

Fix the following problems:
- Always free vertex next hops on `vertex_parent_free`
- Signalize failure on `ospf_spf_add_parent` when parent already exists
  so the caller has the chance to `free()` any allocated resources.
- Don't reuse vertex next hops without the reference count logic in
  `ospf_nexthop_calculation`. Instead allocate a new copy so it can be
  `free()`d later without complications

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

1 files changed, 48 insertions, 17 deletions

diff --git a/ospfd/ospf_spf.c b/ospfd/ospf_spf.c
index 24ca4dcbf..4f60ce22a 100644
--- a/ospfd/ospf_spf.c
+++ b/ospfd/ospf_spf.c
@@ -181,8 +181,10 @@ static struct vertex_parent *vertex_parent_new(struct vertex *v, int backlink,
 	return new;
 }
 
-static void vertex_parent_free(void *p)
+static void vertex_parent_free(struct vertex_parent *p)
 {
+	vertex_nexthop_free(p->local_nexthop);
+	vertex_nexthop_free(p->nexthop);
 	XFREE(MTYPE_OSPF_VERTEX_PARENT, p);
 }
 
@@ -205,7 +207,7 @@ static struct vertex *ospf_vertex_new(struct ospf_area *area,
 	new->lsa = lsa->data;
 	new->children = list_new();
 	new->parents = list_new();
-	new->parents->del = vertex_parent_free;
+	new->parents->del = (void (*)(void *))vertex_parent_free;
 	new->parents->cmp = vertex_parent_cmp;
 	new->lsa_p = lsa;
 
@@ -348,7 +350,7 @@ static struct vertex *ospf_spf_vertex_copy(struct vertex *vertex)
 
 	memcpy(copy, vertex, sizeof(struct vertex));
 	copy->parents = list_new();
-	copy->parents->del = vertex_parent_free;
+	copy->parents->del = (void (*)(void *))vertex_parent_free;
 	copy->parents->cmp = vertex_parent_cmp;
 	copy->children = list_new();
 
@@ -685,11 +687,15 @@ static void ospf_spf_flush_parents(struct vertex *w)
 /*
  * Consider supplied next-hop for inclusion to the supplied list of
  * equal-cost next-hops, adjust list as necessary.
+ *
+ * Returns vertex parent pointer if created otherwise `NULL` if it already
+ * exists.
  */
-static void ospf_spf_add_parent(struct vertex *v, struct vertex *w,
-				struct vertex_nexthop *newhop,
-				struct vertex_nexthop *newlhop,
-				unsigned int distance)
+static struct vertex_parent *ospf_spf_add_parent(struct vertex *v,
+						 struct vertex *w,
+						 struct vertex_nexthop *newhop,
+						 struct vertex_nexthop *newlhop,
+						 unsigned int distance)
 {
 	struct vertex_parent *vp, *wp;
 	struct listnode *node;
@@ -735,7 +741,8 @@ static void ospf_spf_add_parent(struct vertex *v, struct vertex *w,
 				zlog_debug(
 					"%s: ... nexthop already on parent list, skipping add",
 					__func__);
-			return;
+
+			return NULL;
 		}
 	}
 
@@ -743,7 +750,7 @@ static void ospf_spf_add_parent(struct vertex *v, struct vertex *w,
 			       newlhop);
 	listnode_add_sort(w->parents, vp);
 
-	return;
+	return vp;
 }
 
 static int match_stub_prefix(struct lsa_header *lsa, struct in_addr v_link_addr,
@@ -981,8 +988,12 @@ static unsigned int ospf_nexthop_calculation(struct ospf_area *area,
 					memcpy(lnh, nh,
 					       sizeof(struct vertex_nexthop));
 
-					ospf_spf_add_parent(v, w, nh, lnh,
-							    distance);
+					if (ospf_spf_add_parent(v, w, nh, lnh,
+								distance) ==
+					    NULL) {
+						vertex_nexthop_free(nh);
+						vertex_nexthop_free(lnh);
+					}
 					return 1;
 				} else
 					zlog_info(
@@ -1021,8 +1032,13 @@ static unsigned int ospf_nexthop_calculation(struct ospf_area *area,
 					memcpy(lnh, nh,
 					       sizeof(struct vertex_nexthop));
 
-					ospf_spf_add_parent(v, w, nh, lnh,
-							    distance);
+					if (ospf_spf_add_parent(v, w, nh, lnh,
+								distance) ==
+					    NULL) {
+						vertex_nexthop_free(nh);
+						vertex_nexthop_free(lnh);
+					}
+
 					return 1;
 				} else
 					zlog_info(
@@ -1045,7 +1061,12 @@ static unsigned int ospf_nexthop_calculation(struct ospf_area *area,
 			lnh = vertex_nexthop_new();
 			memcpy(lnh, nh, sizeof(struct vertex_nexthop));
 
-			ospf_spf_add_parent(v, w, nh, lnh, distance);
+			if (ospf_spf_add_parent(v, w, nh, lnh, distance) ==
+			    NULL) {
+				vertex_nexthop_free(nh);
+				vertex_nexthop_free(lnh);
+			}
+
 			return 1;
 		}
 	} /* end V is the root */
@@ -1088,8 +1109,12 @@ static unsigned int ospf_nexthop_calculation(struct ospf_area *area,
 					       sizeof(struct vertex_nexthop));
 
 					added = 1;
-					ospf_spf_add_parent(v, w, nh, lnh,
-							    distance);
+					if (ospf_spf_add_parent(v, w, nh, lnh,
+								distance) ==
+					    NULL) {
+						vertex_nexthop_free(nh);
+						vertex_nexthop_free(lnh);
+					}
 				}
 				/*
 				 * Note lack of return is deliberate. See next
@@ -1150,7 +1175,13 @@ static unsigned int ospf_nexthop_calculation(struct ospf_area *area,
 			lnh = NULL;
 		}
 
-		ospf_spf_add_parent(v, w, vp->nexthop, lnh, distance);
+		nh = vertex_nexthop_new();
+		*nh = *vp->nexthop;
+
+		if (ospf_spf_add_parent(v, w, nh, lnh, distance) == NULL) {
+			vertex_nexthop_free(nh);
+			vertex_nexthop_free(lnh);
+		}
 	}
 
 	return added;