lib: fix route map crash on prefix list removal

Changes:
- Refactor list entry deletion to use a function that properly notifies
  route map on deletion (fixes a heap-use-after-free).
- Prefix list entry wild card sets `le` to maximum IP mask value and
  `any` is a boolean.
- Fix prefix list trie removal order (in `prefix_list_entry_update_start`).
- Let only the `any` callback change the value of field `any`.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

lib/filter_nb.c

@@ -98,6 +98,19 @@ prefix_list_length_validate(const struct lyd_node *dnode)
 	return NB_ERR_VALIDATION;
 }
 
+/**
+ * Sets prefix list entry to blank value.
+ *
+ * \param[out] ple prefix list entry to modify.
+ */
+static void prefix_list_entry_set_empty(struct prefix_list_entry *ple)
+{
+	ple->any = false;
+	memset(&ple->prefix, 0, sizeof(ple->prefix));
+	ple->ge = 0;
+	ple->le = 0;
+}
+
 /*
  * XPath: /frr-filter:lib/access-list-legacy
  */
@@ -836,8 +849,8 @@ static int lib_prefix_list_entry_create(struct nb_cb_create_args *args)
 	pl = nb_running_get_entry(args->dnode, NULL, true);
 	ple = prefix_list_entry_new();
 	ple->pl = pl;
-	ple->any = 1;
 	ple->seq = yang_dnode_get_uint32(args->dnode, "./sequence");
+	prefix_list_entry_set_empty(ple);
 	nb_running_set_entry(args->dnode, ple);
 
 	return NB_OK;
@@ -852,7 +865,7 @@ static int lib_prefix_list_entry_destroy(struct nb_cb_destroy_args *args)
 
 	ple = nb_running_unset_entry(args->dnode);
 	if (ple->installed)
-		prefix_list_entry_delete(ple->pl, ple, 0);
+		prefix_list_entry_delete2(ple);
 	else
 		prefix_list_entry_free(ple);
 
@@ -904,7 +917,6 @@ lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args)
 	prefix_list_entry_update_start(ple);
 
 	yang_dnode_get_prefix(&ple->prefix, args->dnode, NULL);
-	ple->any = 0;
 
 	/* Finish prefix entry update procedure. */
 	prefix_list_entry_update_finish(ple);
@@ -926,7 +938,6 @@ lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args)
 	prefix_list_entry_update_start(ple);
 
 	memset(&ple->prefix, 0, sizeof(ple->prefix));
-	ple->any = 1;
 
 	/* Finish prefix entry update procedure. */
 	prefix_list_entry_update_finish(ple);
@@ -1087,6 +1098,7 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy(
 static int lib_prefix_list_entry_any_create(struct nb_cb_create_args *args)
 {
 	struct prefix_list_entry *ple;
+	int type;
 
 	if (args->event != NB_EV_APPLY)
 		return NB_OK;
@@ -1096,8 +1108,24 @@ static int lib_prefix_list_entry_any_create(struct nb_cb_create_args *args)
 	/* Start prefix entry update procedure. */
 	prefix_list_entry_update_start(ple);
 
+	ple->any = true;
+
+	/* Fill prefix struct from scratch. */
 	memset(&ple->prefix, 0, sizeof(ple->prefix));
-	ple->any = 1;
+
+	type = yang_dnode_get_enum(args->dnode, "../../type");
+	switch (type) {
+	case 0: /* ipv4 */
+		ple->prefix.family = AF_INET;
+		ple->ge = 0;
+		ple->le = IPV4_MAX_BITLEN;
+		break;
+	case 1: /* ipv6 */
+		ple->prefix.family = AF_INET6;
+		ple->ge = 0;
+		ple->le = IPV6_MAX_BITLEN;
+		break;
+	}
 
 	/* Finish prefix entry update procedure. */
 	prefix_list_entry_update_finish(ple);
@@ -1117,8 +1145,7 @@ static int lib_prefix_list_entry_any_destroy(struct nb_cb_destroy_args *args)
 	/* Start prefix entry update procedure. */
 	prefix_list_entry_update_start(ple);
 
-	memset(&ple->prefix, 0, sizeof(ple->prefix));
+	prefix_list_entry_set_empty(ple);
-	ple->any = 1;
 
 	/* Finish prefix entry update procedure. */
 	prefix_list_entry_update_finish(ple);

lib/plist.c

@@ -661,6 +661,8 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple)
 	if (!ple->installed)
 		return;
 
+	prefix_list_trie_del(pl, ple);
+
 	/* List manipulation: shameless copy from `prefix_list_entry_delete`. */
 	if (ple->prev)
 		ple->prev->next = ple->next;
@@ -671,11 +673,17 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple)
 	else
 		pl->tail = ple->prev;
 
-	prefix_list_trie_del(pl, ple);
 	route_map_notify_pentry_dependencies(pl->name, ple,
 					     RMAP_EVENT_PLIST_DELETED);
 	pl->count--;
 
+	route_map_notify_dependencies(pl->name, RMAP_EVENT_PLIST_DELETED);
+	if (pl->master->delete_hook)
+		(*pl->master->delete_hook)(pl);
+
+	if (pl->head || pl->tail || pl->desc)
+		pl->master->recent = pl;
+
 	ple->installed = false;
 }
 
@@ -695,6 +703,14 @@ void prefix_list_entry_update_finish(struct prefix_list_entry *ple)
 	if (ple->installed)
 		return;
 
+	/*
+	 * Check if the entry is installable:
+	 * We can only install entry if at least the prefix is provided (IPv4
+	 * or IPv6).
+	 */
+	if (ple->prefix.family != AF_INET && ple->prefix.family != AF_INET6)
+		return;
+
 	/* List manipulation: shameless copy from `prefix_list_entry_add`. */
 	if (pl->tail && ple->seq > pl->tail->seq)
 		point = NULL;
@@ -742,6 +758,21 @@ void prefix_list_entry_update_finish(struct prefix_list_entry *ple)
 	ple->installed = true;
 }
 
+/**
+ * Same as `prefix_list_entry_delete` but without `free()`ing the list if its
+ * empty.
+ *
+ * \param[in] ple prefix list entry.
+ */
+void prefix_list_entry_delete2(struct prefix_list_entry *ple)
+{
+	/* Does the boiler plate list removal and entry removal notification. */
+	prefix_list_entry_update_start(ple);
+
+	/* Effective `free()` memory. */
+	prefix_list_entry_free(ple);
+}
+
 /* Return string of prefix_list_type. */
 static const char *prefix_list_type_str(struct prefix_list_entry *pentry)
 {

lib/plist_int.h

@@ -78,6 +78,7 @@ struct prefix_list_entry {
 };
 
 extern void prefix_list_entry_free(struct prefix_list_entry *pentry);
+extern void prefix_list_entry_delete2(struct prefix_list_entry *ple);
 extern void prefix_list_entry_update_start(struct prefix_list_entry *ple);
 extern void prefix_list_entry_update_finish(struct prefix_list_entry *ple);