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>

diff --git a/ospfd/ospf_spf.c b/ospfd/ospf_spf.c
index 24ca4dcbf4193de46a9833fc995e88b33caaec56..4f60ce22a9ecbbb5200be837e72df77d3265749e 100644 (file)
--- 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;