zebra: Fix memory leaks and use after frees in nhg's on shutdown

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>

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index bd793ed5dd1a3a6486fa0c9b8837ca7d01de1f76..b2d93477d435692873ccab6328fc6043f11d2976 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -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)

diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
index 6d2ab248f9a7d6078967e4e9c81712a47cba95b5..62f71f943fddd17a2ef9dc30df6de88f1538e3ff 100644 (file)
--- a/zebra/zebra_nhg.h
+++ b/zebra/zebra_nhg.h
@@ -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

diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c
index f7ad30b41fb337b76694c277a5a08135816e42af..24e71b4a8b8266d31a676147a0e9e86eecddbd38 100644 (file)
--- a/zebra/zebra_router.c
+++ b/zebra/zebra_router.c
@@ -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);