From 54a35ff48b600cd59b715b6e5aea4e69de1b995f Mon Sep 17 00:00:00 2001
From: Rafael Zalamena <rzalamena@opensourcerouting.org>
Date: Mon, 14 Oct 2019 23:29:19 -0300
Subject: [PATCH] lib: fix route map northbound memory leak

Keep a list of hook contexts used by northbound so we don't lose the
pointer when free()ing the route map index entry data.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
---
 lib/routemap.c            |  5 +++++
 lib/routemap.h            | 11 +++++++++++
 lib/routemap_northbound.c | 41 +++++++++++++++++++++++++++++----------
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/lib/routemap.c b/lib/routemap.c
index 5369fa771f..912cf28202 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -909,6 +909,7 @@ static struct route_map_index *route_map_index_new(void)
 
 	new = XCALLOC(MTYPE_ROUTE_MAP_INDEX, sizeof(struct route_map_index));
 	new->exitpolicy = RMAP_EXIT; /* Default to Cisco-style */
+	TAILQ_INIT(&new->rhclist);
 	QOBJ_REG(new, route_map_index);
 	return new;
 }
@@ -924,6 +925,10 @@ void route_map_index_delete(struct route_map_index *index, int notify)
 		zlog_debug("Deleting route-map %s sequence %d",
 			   index->map->name, index->pref);
 
+	/* Free route map northbound hook contexts. */
+	while (!TAILQ_EMPTY(&index->rhclist))
+		routemap_hook_context_free(TAILQ_FIRST(&index->rhclist));
+
 	/* Free route match. */
 	while ((rule = index->match_list.head) != NULL)
 		route_map_rule_delete(&index->match_list, rule);
diff --git a/lib/routemap.h b/lib/routemap.h
index 70e150c981..05c958967c 100644
--- a/lib/routemap.h
+++ b/lib/routemap.h
@@ -162,6 +162,9 @@ struct route_map_rule_list {
 	struct route_map_rule *tail;
 };
 
+/* Forward struct declaration: the complete can be found later this file. */
+struct routemap_hook_context;
+
 /* Route map index structure. */
 struct route_map_index {
 	struct route_map *map;
@@ -194,6 +197,9 @@ struct route_map_index {
 	uint64_t applied;
 	uint64_t applied_clear;
 
+	/* List of match/sets contexts. */
+	TAILQ_HEAD(, routemap_hook_context) rhclist;
+
 	QOBJ_FIELDS
 };
 DECLARE_QOBJ_TYPE(route_map_index)
@@ -660,6 +666,7 @@ struct routemap_hook_context {
 	route_map_event_t rhc_event;
 	routemap_set_hook_fun rhc_shook;
 	routemap_match_hook_fun rhc_mhook;
+	TAILQ_ENTRY(routemap_hook_context) rhc_entry;
 };
 
 int lib_route_map_entry_match_destroy(enum nb_event event,
@@ -667,6 +674,10 @@ int lib_route_map_entry_match_destroy(enum nb_event event,
 int lib_route_map_entry_set_destroy(enum nb_event event,
 				    const struct lyd_node *dnode);
 
+struct routemap_hook_context *
+routemap_hook_context_insert(struct route_map_index *rmi);
+void routemap_hook_context_free(struct routemap_hook_context *rhc);
+
 extern const struct frr_yang_module_info frr_route_map_info;
 
 /* routemap_cli.c */
diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c
index b9ac01e865..3173a708e4 100644
--- a/lib/routemap_northbound.c
+++ b/lib/routemap_northbound.c
@@ -74,6 +74,30 @@ int lib_route_map_entry_set_destroy(enum nb_event event,
 	return NB_OK;
 }
 
+/*
+ * Auxiliary hook context list manipulation functions.
+ */
+struct routemap_hook_context *
+routemap_hook_context_insert(struct route_map_index *rmi)
+{
+	struct routemap_hook_context *rhc;
+
+	rhc = XCALLOC(MTYPE_TMP, sizeof(*rhc));
+	rhc->rhc_rmi = rmi;
+	TAILQ_INSERT_TAIL(&rmi->rhclist, rhc, rhc_entry);
+
+	return rhc;
+}
+
+void
+routemap_hook_context_free(struct routemap_hook_context *rhc)
+{
+	struct route_map_index *rmi = rhc->rhc_rmi;
+
+	TAILQ_REMOVE(&rmi->rhclist, rhc, rhc_entry);
+	XFREE(MTYPE_TMP, rhc);
+}
+
 /*
  * XPath: /frr-route-map:lib/route-map
  */
@@ -436,20 +460,17 @@ lib_route_map_entry_match_condition_create(enum nb_event event,
 					   union nb_resource *resource)
 {
 	struct routemap_hook_context *rhc;
+	struct route_map_index *rmi;
 
 	switch (event) {
 	case NB_EV_VALIDATE:
+	case NB_EV_PREPARE:
+	case NB_EV_ABORT:
 		/* NOTHING */
 		break;
-	case NB_EV_PREPARE:
-		resource->ptr = XCALLOC(MTYPE_TMP, sizeof(*rhc));
-		break;
-	case NB_EV_ABORT:
-		XFREE(MTYPE_TMP, resource->ptr);
-		break;
 	case NB_EV_APPLY:
-		rhc = resource->ptr;
-		rhc->rhc_rmi = nb_running_get_entry(dnode, NULL, true);
+		rmi = nb_running_get_entry(dnode, NULL, true);
+		rhc = routemap_hook_context_insert(rmi);
 		nb_running_set_entry(dnode, rhc);
 		break;
 	}
@@ -469,7 +490,7 @@ lib_route_map_entry_match_condition_destroy(enum nb_event event,
 
 	rv = lib_route_map_entry_match_destroy(event, dnode);
 	rhc = nb_running_unset_entry(dnode);
-	XFREE(MTYPE_TMP, rhc);
+	routemap_hook_context_free(rhc);
 
 	return rv;
 }
@@ -893,7 +914,7 @@ static int lib_route_map_entry_set_action_destroy(enum nb_event event,
 
 	rv = lib_route_map_entry_set_destroy(event, dnode);
 	rhc = nb_running_unset_entry(dnode);
-	XFREE(MTYPE_TMP, rhc);
+	routemap_hook_context_free(rhc);
 
 	return rv;
 }