From 7d3280f1dc9be4d28aa20a870ea933b91c933524 Mon Sep 17 00:00:00 2001 From: vivek Date: Wed, 18 May 2016 14:08:55 -0700 Subject: [PATCH] Quagga: Make sure order of route-maps in list and hash table matches MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel Walton Reviewed-by: Donald Sharp Reviewed-by: Shrijeet Mukherjee Ticket: CM-10023 Reviewed By: CCR-4747 Testing Done: manual, bgp-smoke --- lib/routemap.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/routemap.c b/lib/routemap.c index 1a31271b17..d8d1872d65 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) -- 2.39.5