Skip to content

Commit

Permalink
zebra: Fix memory leaks and use after frees in nhg's on shutdown
Browse files Browse the repository at this point in the history
Fixup both memory leaks as well as use after free's in nhg's
on shutdown.

This approach is effectively just iterating through all the
hash items and directly just freeing the memory instead
of handling ref counts or cross references.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
  • Loading branch information
donaldsharp committed Aug 5, 2022
1 parent 34a67a7 commit d579510
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
55 changes: 53 additions & 2 deletions zebra/zebra_nhg.c
Original file line number Diff line number Diff line change
Expand Up @@ -1621,10 +1621,61 @@ void zebra_nhg_free(struct nhg_hash_entry *nhe)
XFREE(MTYPE_NHG, nhe);
}

/*
* Let's just drop the memory associated with each item
*/
void zebra_nhg_hash_free(void *p)
{
zebra_nhg_release_all_deps((struct nhg_hash_entry *)p);
zebra_nhg_free((struct nhg_hash_entry *)p);
struct nhg_hash_entry *nhe = p;

if (IS_ZEBRA_DEBUG_NHG_DETAIL) {
/* Group or singleton? */
if (nhe->nhg.nexthop && nhe->nhg.nexthop->next)
zlog_debug("%s: nhe %p (%u), refcnt %d", __func__, nhe,
nhe->id, nhe->refcnt);
else
zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv",
__func__, nhe, nhe, nhe->refcnt,
nhe->nhg.nexthop);
}

THREAD_OFF(nhe->timer);

nexthops_free(nhe->nhg.nexthop);

XFREE(MTYPE_NHG, nhe);
}

/*
* On cleanup there are nexthop groups that have not
* been resolved at all( a nhe->id of 0 ). As such
* zebra needs to clean up the memory associated with
* those entries.
*/
void zebra_nhg_hash_free_zero_id(struct hash_bucket *b, void *arg)
{
struct nhg_hash_entry *nhe = b->data;
struct nhg_connected *dep;

while ((dep = nhg_connected_tree_pop(&nhe->nhg_depends))) {
if (dep->nhe->id == 0)
zebra_nhg_hash_free(dep->nhe);

nhg_connected_free(dep);
}

while ((dep = nhg_connected_tree_pop(&nhe->nhg_dependents)))
nhg_connected_free(dep);

if (nhe->backup_info && nhe->backup_info->nhe->id == 0) {
while ((dep = nhg_connected_tree_pop(
&nhe->backup_info->nhe->nhg_depends)))
nhg_connected_free(dep);

zebra_nhg_hash_free(nhe->backup_info->nhe);

XFREE(MTYPE_NHG, nhe->backup_info);
}
}

static void zebra_nhg_timer(struct thread *thread)
Expand Down
1 change: 1 addition & 0 deletions zebra/zebra_nhg.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ struct nhg_hash_entry *zebra_nhg_alloc(void);
void zebra_nhg_free(struct nhg_hash_entry *nhe);
/* In order to clear a generic hash, we need a generic api, sigh. */
void zebra_nhg_hash_free(void *p);
void zebra_nhg_hash_free_zero_id(struct hash_bucket *b, void *arg);

/* Init an nhe, for use in a hash lookup for example. There's some fuzziness
* if the nhe represents only a single nexthop, so we try to capture that
Expand Down
1 change: 1 addition & 0 deletions zebra/zebra_router.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ void zebra_router_terminate(void)
zebra_neigh_terminate();

/* Free NHE in ID table only since it has unhashable entries as well */
hash_iterate(zrouter.nhgs_id, zebra_nhg_hash_free_zero_id, NULL);
hash_clean(zrouter.nhgs_id, zebra_nhg_hash_free);
hash_free(zrouter.nhgs_id);
hash_clean(zrouter.nhgs, NULL);
Expand Down

0 comments on commit d579510

Please sign in to comment.