Quagga: Make sure order of route-maps in list and hash table matches

Quick create/delete actions on a route-map can result in the same route-map
entity having multiple entries created for it — because BGP hasn't run the
update processing to complete prior delete action. The route-map is present
in both a hash table as well as a linked list and the order in each is
different. This can lead to problems when the BGP route-map update processing
runs and finds the same route-map entity present for deletion multiple times.
For example, while processing instance-2 of rmap-A, the code may end up
freeing the hash bucket corresponding to instance-1 of rmap-A.

The fix works by ensuring the list is ordered the same way as the hash
buckets.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Shrijeet Mukherjee <shm@cumulusnetworks.com>

Ticket: CM-10023
Reviewed By: CCR-4747
Testing Done: manual, bgp-smoke

1 files changed, 17 insertions, 7 deletions

diff --git a/lib/routemap.c b/lib/routemap.c
index 1a31271b1..d8d1872d6 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -163,13 +163,23 @@ route_map_add (const char *name)
   /* Add map to the hash */
   hash_get(route_map_master_hash, map, hash_alloc_intern);
 
-  map->next = NULL;
-  map->prev = list->tail;
-  if (list->tail)
-    list->tail->next = map;
-  else
-    list->head = map;
-  list->tail = map;
+  /* Add new entry to the head of the list to match how it is added in the
+   * hash table. This is to ensure that if the same route-map has been
+   * created more than once and then marked for deletion (which can happen
+   * if prior deletions haven't completed as BGP hasn't yet done the
+   * route-map processing), the order of the entities is the same in both
+   * the list and the hash table. Otherwise, since there is nothing to
+   * distinguish between the two entries, the wrong entry could get freed.
+   * TODO: This needs to be re-examined to handle it better - e.g., revive
+   * a deleted entry if the route-map is created again.
+   */
+  map->prev = NULL;
+  map->next = list->head;
+  if (list->head)
+    list->head->prev = map;
+  list->head = map;
+  if (!list->tail)
+    list->tail = map;
 
   /* Execute hook. */
   if (route_map_master.add_hook)