eigrpd: Fix massive memory leak

Each call into eigrp_topology_get_successors would leak the list
created.  As routes worked their way through the FSM we would
leak memory left and right.

Modify the eigrp_topology_get_successor to return NULL when
there are no SUCCESORS.

Clean up some dead code.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

diff --git a/eigrpd/eigrp_fsm.c b/eigrpd/eigrp_fsm.c
index 0277dc830f162321a3500fb55b73f97608f653df..06ff09aa1350d59ec821cc8419f0fd246933a702 100644 (file)
--- a/eigrpd/eigrp_fsm.c
+++ b/eigrpd/eigrp_fsm.c
@@ -346,6 +346,9 @@ int eigrp_fsm_event_nq_fcn(struct eigrp_fsm_action_message *msg) {
        struct eigrp *eigrp = msg->eigrp;
        struct eigrp_prefix_entry *prefix = msg->prefix;
        struct list *successors = eigrp_topology_get_successor(prefix);
+
+       assert(successors);  // If this is NULL we have shit the bed, fun huh?
+
        prefix->state = EIGRP_FSM_STATE_ACTIVE_1;
        prefix->rdistance = prefix->distance = prefix->fdistance =
                        ((struct eigrp_neighbor_entry *) successors->head->data)->distance;
@@ -359,6 +362,8 @@ int eigrp_fsm_event_nq_fcn(struct eigrp_fsm_action_message *msg) {
                eigrp_fsm_event_lr(msg); //in the case that there are no more neighbors left
        }
 
+       list_delete(successors);
+
        return 1;
 }
 
@@ -366,6 +371,9 @@ int eigrp_fsm_event_q_fcn(struct eigrp_fsm_action_message *msg) {
        struct eigrp *eigrp = msg->eigrp;
        struct eigrp_prefix_entry *prefix = msg->prefix;
        struct list *successors = eigrp_topology_get_successor(prefix);
+
+       assert(successors);  // If this is NULL somebody poked us in the eye.
+
        prefix->state = EIGRP_FSM_STATE_ACTIVE_3;
        prefix->rdistance = prefix->distance = prefix->fdistance =
                        ((struct eigrp_neighbor_entry *) successors->head->data)->distance;
@@ -378,6 +386,8 @@ int eigrp_fsm_event_q_fcn(struct eigrp_fsm_action_message *msg) {
                        eigrp_fsm_event_lr(msg); //in the case that there are no more neighbors left
                }
 
+       list_delete(successors);
+
        return 1;
 }
 
@@ -413,15 +423,21 @@ int eigrp_fsm_event_lr(struct eigrp_fsm_action_message *msg) {
        struct eigrp *eigrp = msg->eigrp;
        struct eigrp_prefix_entry *prefix = msg->prefix;
        prefix->fdistance =
-                       prefix->distance =
-                                       prefix->rdistance =
-                                                       ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->distance;
+         prefix->distance =
+         prefix->rdistance =
+         ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->distance;
        prefix->reported_metric =
-                       ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->total_metric;
-       if (prefix->state == EIGRP_FSM_STATE_ACTIVE_3)
-               eigrp_send_reply(
-                               ((struct eigrp_neighbor_entry *) (eigrp_topology_get_successor(
-                                               prefix)->head->data))->adv_router, prefix);
+         ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->total_metric;
+
+       if (prefix->state == EIGRP_FSM_STATE_ACTIVE_3) {
+               struct list *successors = eigrp_topology_get_successor(prefix);
+
+               assert(successors);  // It's like Napolean and Waterloo
+
+               eigrp_send_reply(((struct eigrp_neighbor_entry *)successors->head->data)->adv_router, prefix);
+               list_delete(successors);
+       }
+
        prefix->state = EIGRP_FSM_STATE_PASSIVE;
        prefix->req_action |= EIGRP_FSM_NEED_UPDATE;
        listnode_add(eigrp->topology_changes_internalIPV4,prefix);
@@ -433,17 +449,19 @@ int eigrp_fsm_event_lr(struct eigrp_fsm_action_message *msg) {
 }
 
 int eigrp_fsm_event_dinc(struct eigrp_fsm_action_message *msg) {
+       struct list *successors = eigrp_topology_get_successor(msg->prefix);
+
+       assert(successors);  // Trump and his big hands
 
        msg->prefix->state =
-                       msg->prefix->state == EIGRP_FSM_STATE_ACTIVE_1 ?
-                                       EIGRP_FSM_STATE_ACTIVE_0 : EIGRP_FSM_STATE_ACTIVE_2;
-       msg->prefix->distance =
-                       ((struct eigrp_neighbor_entry *) (eigrp_topology_get_successor(
-                                       msg->prefix)->head->data))->distance;
+               msg->prefix->state == EIGRP_FSM_STATE_ACTIVE_1 ?
+                                     EIGRP_FSM_STATE_ACTIVE_0 : EIGRP_FSM_STATE_ACTIVE_2;
+       msg->prefix->distance = ((struct eigrp_neighbor_entry *)successors->head->data)->distance;
        if (!msg->prefix->rij->count) {
                (*(NSM[msg->prefix->state][eigrp_get_fsm_event(msg)].func))(msg);
        }
 
+       list_delete(successors);
        return 1;
 }
 
@@ -452,17 +470,22 @@ int eigrp_fsm_event_lr_fcs(struct eigrp_fsm_action_message *msg) {
        struct eigrp_prefix_entry *prefix = msg->prefix;
        prefix->state = EIGRP_FSM_STATE_PASSIVE;
        prefix->distance =
-                       prefix->rdistance =
-                                       ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->distance;
+               prefix->rdistance =
+               ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->distance;
        prefix->reported_metric =
                        ((struct eigrp_neighbor_entry *) (prefix->entries->head->data))->total_metric;
        prefix->fdistance =
                        prefix->fdistance > prefix->distance ?
                                        prefix->distance : prefix->fdistance;
-       if (prefix->state == EIGRP_FSM_STATE_ACTIVE_2)
-               eigrp_send_reply(
-                               ((struct eigrp_neighbor_entry *) (eigrp_topology_get_successor(
-                                               prefix)->head->data))->adv_router, prefix);
+       if (prefix->state == EIGRP_FSM_STATE_ACTIVE_2) {
+               struct list *successors = eigrp_topology_get_successor(prefix);
+
+               assert(successors);  // Having a spoon and all you need is a knife
+
+               eigrp_send_reply(((struct eigrp_neighbor_entry *)successors->head->data)->adv_router, prefix);
+
+               list_delete(successors);
+       }
        prefix->req_action |= EIGRP_FSM_NEED_UPDATE;
        listnode_add(eigrp->topology_changes_internalIPV4,prefix);
        eigrp_topology_update_node_flags(prefix);
@@ -475,12 +498,15 @@ int eigrp_fsm_event_lr_fcs(struct eigrp_fsm_action_message *msg) {
 int eigrp_fsm_event_lr_fcn(struct eigrp_fsm_action_message *msg) {
        struct eigrp *eigrp = msg->eigrp;
        struct eigrp_prefix_entry *prefix = msg->prefix;
+       struct list *successors = eigrp_topology_get_successor(prefix);
+
+       assert(successors);  // Routing without a stack
+
        prefix->state =
-                       prefix->state == EIGRP_FSM_STATE_ACTIVE_0 ?
-                                       EIGRP_FSM_STATE_ACTIVE_1 : EIGRP_FSM_STATE_ACTIVE_3;
+               prefix->state == EIGRP_FSM_STATE_ACTIVE_0 ?
+                                  EIGRP_FSM_STATE_ACTIVE_1 : EIGRP_FSM_STATE_ACTIVE_3;
        struct eigrp_neighbor_entry *best_successor =
-                       ((struct eigrp_neighbor_entry *) (eigrp_topology_get_successor(
-                                       prefix)->head->data));
+                       ((struct eigrp_neighbor_entry *) (successors->head->data));
        prefix->rdistance = prefix->distance = best_successor->distance;
        prefix->reported_metric = best_successor->total_metric;
        if (eigrp_nbr_count_get()) {
@@ -490,13 +516,20 @@ int eigrp_fsm_event_lr_fcn(struct eigrp_fsm_action_message *msg) {
                eigrp_fsm_event_lr(msg); //in the case that there are no more neighbors left
        }
 
+       list_delete(successors);
+
        return 1;
 }
 
 int eigrp_fsm_event_qact(struct eigrp_fsm_action_message *msg) {
+       struct list *successors = eigrp_topology_get_successor(msg->prefix);
+
+       assert(successors);  // Cats and no Dogs
+
        msg->prefix->state = EIGRP_FSM_STATE_ACTIVE_2;
        msg->prefix->distance =
-                       ((struct eigrp_neighbor_entry *) (eigrp_topology_get_successor(
-                                       msg->prefix)->head->data))->distance;
+                       ((struct eigrp_neighbor_entry *) (successors->head->data))->distance;
+
+       list_delete(successors);
        return 1;
 }

diff --git a/eigrpd/eigrp_topology.c b/eigrpd/eigrp_topology.c
index e5adcc4db52c60d0d9b7962d6c47a3be420e68a3..7015d595355302e322d07b8f9ef159571f2aa262 100644 (file)
--- a/eigrpd/eigrp_topology.c
+++ b/eigrpd/eigrp_topology.c
@@ -316,23 +316,7 @@ eigrp_topology_table_lookup_ipv4(struct list *topology_table,
 
   return NULL;
 }
-/* TODO
- struct eigrp_prefix_entry *
- eigrp_topology_table_lookup_ipv6 (struct list *topology_table,
- struct prefix_ipv6 * address)
- {
- struct eigrp_prefix_entry *data;
- struct listnode *node, *nnode;
- for (ALL_LIST_ELEMENTS (topology_table, node, nnode, data))
- {
-
- if (comparison)
- return data;
- }
-
- return NULL;
- }
- */
+
 struct list *
 eigrp_topology_get_successor(struct eigrp_prefix_entry *table_node)
 {
@@ -348,6 +332,15 @@ eigrp_topology_get_successor(struct eigrp_prefix_entry *table_node)
         }
     }
 
+  /*
+   * If we have no successors return NULL
+   */
+  if (!successors->count)
+    {
+      list_delete(successors);
+      successors = NULL;
+    }
+
   return successors;
 }
 

diff --git a/eigrpd/eigrp_topology.h b/eigrpd/eigrp_topology.h
index 9f1569ea6b8616e3786ad085c6db21f9a9d952ca..33b5c893c7f0546beabdeb9756ae274e24eff486 100644 (file)
--- a/eigrpd/eigrp_topology.h
+++ b/eigrpd/eigrp_topology.h
@@ -58,18 +58,5 @@ extern int eigrp_topology_update_distance ( struct eigrp_fsm_action_message *);
 extern void eigrp_update_routing_table(struct eigrp_prefix_entry *);
 extern void eigrp_topology_neighbor_down(struct eigrp *, struct eigrp_neighbor *);
 extern void eigrp_update_topology_table_prefix(struct list *, struct eigrp_prefix_entry * );
-//extern int eigrp_topology_get_successor_count (struct eigrp_prefix_entry *);
-/* Set all stats to -1 (LSA_SPF_NOT_EXPLORED). */
-/*extern void eigrp_lsdb_clean_stat (struct eigrp_lsdb *lsdb);
-extern struct eigrp_lsa *eigrp_lsdb_lookup_by_id (struct eigrp_lsdb *, u_char,
-                                        struct in_addr, struct in_addr);
-extern struct eigrp_lsa *eigrp_lsdb_lookup_by_id_next (struct eigrp_lsdb *, u_char,
-                                             struct in_addr, struct in_addr,
-                                             int);
-extern unsigned long eigrp_lsdb_count_all (struct eigrp_lsdb *);
-extern unsigned long eigrp_lsdb_count (struct eigrp_lsdb *, int);
-extern unsigned long eigrp_lsdb_count_self (struct eigrp_lsdb *, int);
-extern unsigned int eigrp_lsdb_checksum (struct eigrp_lsdb *, int);
-extern unsigned long eigrp_lsdb_isempty (struct eigrp_lsdb *);
-*/
+
 #endif