ospf6d: Fixing memory leak in ospf6_summary_add_aggr_route_and_blackhole.

Problem Statement:
=================
Memory leak in ospf6d.
2022-11-15 02:15:11,569 - ERROR: ==30108== 440 (280 direct, 160 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 17
2022-11-15 02:15:11,569 - ERROR: ==30108==    at 0x4C31FAC: calloc (vg_replace_malloc.c:762)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x4E8A1BF: qcalloc (memory.c:111)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x14337A: ospf6_route_create (ospf6_route.c:462)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x11EE27: ospf6_summary_add_aggr_route_and_blackhole (ospf6_asbr.c:2779)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x11EEDA: ospf6_originate_new_aggr_lsa (ospf6_asbr.c:2816)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x120053: ospf6_handle_external_lsa_origination (ospf6_asbr.c:3659)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x12041E: ospf6_asbr_redistribute_add (ospf6_asbr.c:1547)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x14F3CC: ospf6_zebra_read_route (ospf6_zebra.c:253)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x4EC9B73: zclient_read (zclient.c:2727)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x4EB741B: thread_call (thread.c:1692)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x4E85B17: frr_run (libfrr.c:1068)
2022-11-15 02:15:11,569 - ERROR: ==30108==    by 0x119585: main (ospf6_main.c:228)
2022-11-15 02:15:11,569 - ERROR: ==30108==

RCA:
====
blackhole route was not freed before adding a new one.

Fix:
====
Added a check before allocating new route, to free the old one. Also,
added ospf6_asbr_summary_config_delete in ospf6_delet before freeing
the aggregate route.

Signed-off-by: Manoj Naragund <mnaragund@vmware.com>

diff --git a/ospf6d/ospf6_asbr.c b/ospf6d/ospf6_asbr.c
index 07061b6f578fa493229504c56c3806cf6fa25ed9..0fd6e15ed6045fa596ef72c131ab345626ac8013 100644 (file)
--- a/ospf6d/ospf6_asbr.c
+++ b/ospf6d/ospf6_asbr.c
@@ -1585,7 +1585,11 @@ ospf6_asbr_summary_remove_lsa_and_route(struct ospf6 *ospf6,
                        zlog_debug(
                                "%s: Remove the blackhole route",
                                __func__);
+
                ospf6_zebra_route_update_remove(aggr->route, ospf6);
+               if (aggr->route->route_option)
+                       XFREE(MTYPE_OSPF6_EXTERNAL_INFO,
+                             aggr->route->route_option);
                ospf6_route_delete(aggr->route);
                aggr->route = NULL;
        }
@@ -2736,8 +2740,13 @@ ospf6_summary_add_aggr_route_and_blackhole(struct ospf6 *ospf6,
                                           struct ospf6_external_aggr_rt *aggr)
 {
        struct ospf6_route *rt_aggr;
+       struct ospf6_route *old_rt = NULL;
        struct ospf6_external_info *info;
 
+       /* Check if a route is already present. */
+       if (aggr->route)
+               old_rt = aggr->route;
+
        /* Create summary route and save it. */
        rt_aggr = ospf6_route_create(ospf6);
        rt_aggr->type = OSPF6_DEST_TYPE_NETWORK;
@@ -2756,6 +2765,16 @@ ospf6_summary_add_aggr_route_and_blackhole(struct ospf6 *ospf6,
        /* Add next-hop to Null interface. */
        ospf6_add_route_nexthop_blackhole(rt_aggr);
 
+       /* Free the old route, if any. */
+       if (old_rt) {
+               ospf6_zebra_route_update_remove(old_rt, ospf6);
+
+               if (old_rt->route_option)
+                       XFREE(MTYPE_OSPF6_EXTERNAL_INFO, old_rt->route_option);
+
+               ospf6_route_delete(old_rt);
+       }
+
        ospf6_zebra_route_update_add(rt_aggr, ospf6);
 }
 
@@ -3024,8 +3043,8 @@ static void ospf6_aggr_handle_external_info(void *data)
        (void)ospf6_originate_type5_type7_lsas(rt, ospf6);
 }
 
-static void
-ospf6_asbr_summary_config_delete(struct ospf6 *ospf6, struct route_node *rn)
+void ospf6_asbr_summary_config_delete(struct ospf6 *ospf6,
+                                     struct route_node *rn)
 {
        struct ospf6_external_aggr_rt *aggr = rn->info;
 
@@ -3167,14 +3186,6 @@ void ospf6_external_aggregator_free(struct ospf6_external_aggr_rt *aggr)
                hash_clean(aggr->match_extnl_hash,
                        ospf6_aggr_unlink_external_info);
 
-       if (aggr->route) {
-               if (aggr->route->route_option)
-                       XFREE(MTYPE_OSPF6_EXTERNAL_INFO,
-                             aggr->route->route_option);
-
-               ospf6_route_delete(aggr->route);
-       }
-
        if (IS_OSPF6_DEBUG_AGGR)
                zlog_debug("%s: Release the aggregator Address(%pFX)",
                                                __func__,

diff --git a/ospf6d/ospf6_asbr.h b/ospf6d/ospf6_asbr.h
index 0487bb14c3dae543790ff6c57312eee984188771..0d2a98aebae84dd804200e9152515524b1749177 100644 (file)
--- a/ospf6d/ospf6_asbr.h
+++ b/ospf6d/ospf6_asbr.h
@@ -182,4 +182,6 @@ void ospf6_external_aggregator_free(struct ospf6_external_aggr_rt *aggr);
 void ospf6_unset_all_aggr_flag(struct ospf6 *ospf6);
 void ospf6_fill_aggr_route_details(struct ospf6 *ospf6,
                                   struct ospf6_external_aggr_rt *aggr);
+void ospf6_asbr_summary_config_delete(struct ospf6 *ospf6,
+                                     struct route_node *rn);
 #endif /* OSPF6_ASBR_H */

diff --git a/ospf6d/ospf6_top.c b/ospf6d/ospf6_top.c
index eb89a14cd3829db7ea28dc947589e5d677a18a0a..db45fa5f5c6badf61ac4b0a4a80f39b5524fbe08 100644 (file)
--- a/ospf6d/ospf6_top.c
+++ b/ospf6d/ospf6_top.c
@@ -498,6 +498,7 @@ void ospf6_delete(struct ospf6 *o)
        struct route_node *rn = NULL;
        struct ospf6_area *oa;
        struct vrf *vrf;
+       struct ospf6_external_aggr_rt *aggr;
 
        QOBJ_UNREG(o);
 
@@ -536,8 +537,11 @@ void ospf6_delete(struct ospf6 *o)
        }
 
        for (rn = route_top(o->rt_aggr_tbl); rn; rn = route_next(rn))
-               if (rn->info)
-                       ospf6_external_aggregator_free(rn->info);
+               if (rn->info) {
+                       aggr = rn->info;
+                       ospf6_asbr_summary_config_delete(o, rn);
+                       ospf6_external_aggregator_free(aggr);
+               }
        route_table_finish(o->rt_aggr_tbl);
 
        XFREE(MTYPE_OSPF6_TOP, o->name);