forked from Mirror/frr
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>
This commit is contained in:
parent
f2b72ef474
commit
b3bcfd3dc6
|
@ -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;
|
||||
|
|
Loading…
Reference in a new issue