From a75c1cc19127c543648a57f6f5ea9f3e13666a7f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 19 Aug 2017 13:42:48 -0400 Subject: [PATCH 1/3] eigrpd: Fix memory leak in eigrp_update When we send packets to a nbr, make a duplicate copy as that each packet sent is assumed to be a complete to itself. Also clean up indentation in loop over figuring out what to send. Signed-off-by: Donald Sharp --- eigrpd/eigrp_update.c | 104 +++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/eigrpd/eigrp_update.c b/eigrpd/eigrp_update.c index 5599965f6a..e42f84abc6 100644 --- a/eigrpd/eigrp_update.c +++ b/eigrpd/eigrp_update.c @@ -651,53 +651,47 @@ void eigrp_update_send(struct eigrp_interface *ei) has_tlv = 0; for (ALL_LIST_ELEMENTS(ei->eigrp->topology_changes_internalIPV4, node, nnode, pe)) { - if (pe->req_action & EIGRP_FSM_NEED_UPDATE) { - /* Get destination address from prefix */ - dest_addr = pe->destination_ipv4; - /* - * Filtering - */ - // TODO: Work in progress - /* get list from eigrp process */ - e = eigrp_lookup(); - /* Get access-lists and prefix-lists from process and - * interface */ - alist = e->list[EIGRP_FILTER_OUT]; - plist = e->prefix[EIGRP_FILTER_OUT]; - alist_i = ei->list[EIGRP_FILTER_OUT]; - plist_i = ei->prefix[EIGRP_FILTER_OUT]; + if (!(pe->req_action & EIGRP_FSM_NEED_UPDATE)) + continue; - /* Check if any list fits */ - if ((alist - && access_list_apply(alist, - (struct prefix *)dest_addr) - == FILTER_DENY) - || (plist - && prefix_list_apply(plist, - (struct prefix *)dest_addr) - == PREFIX_DENY) - || (alist_i - && access_list_apply(alist_i, - (struct prefix *)dest_addr) - == FILTER_DENY) - || (plist_i - && prefix_list_apply(plist_i, - (struct prefix *)dest_addr) - == PREFIX_DENY)) { - // pe->reported_metric.delay = EIGRP_MAX_METRIC; - continue; - } else { - length += eigrp_add_internalTLV_to_stream(ep->s, - pe); - has_tlv = 1; - } - /* - * End of filtering - */ + /* Get destination address from prefix */ + dest_addr = pe->destination_ipv4; - /* NULL the pointer */ - dest_addr = NULL; + /* + * Filtering + */ + e = eigrp_lookup(); + /* Get access-lists and prefix-lists from process and + * interface */ + alist = e->list[EIGRP_FILTER_OUT]; + plist = e->prefix[EIGRP_FILTER_OUT]; + alist_i = ei->list[EIGRP_FILTER_OUT]; + plist_i = ei->prefix[EIGRP_FILTER_OUT]; + + /* Check if any list fits */ + if ((alist + && access_list_apply(alist, + (struct prefix *)dest_addr) + == FILTER_DENY) + || (plist + && prefix_list_apply(plist, + (struct prefix *)dest_addr) + == PREFIX_DENY) + || (alist_i + && access_list_apply(alist_i, + (struct prefix *)dest_addr) + == FILTER_DENY) + || (plist_i + && prefix_list_apply(plist_i, + (struct prefix *)dest_addr) + == PREFIX_DENY)) { + // pe->reported_metric.delay = EIGRP_MAX_METRIC; + continue; + } else { + length += eigrp_add_internalTLV_to_stream(ep->s, + pe); + has_tlv = 1; } } @@ -725,14 +719,22 @@ void eigrp_update_send(struct eigrp_interface *ei) ep->sequence_number); for (ALL_LIST_ELEMENTS(ei->nbrs, node, nnode, nbr)) { - if (nbr->state == EIGRP_NEIGHBOR_UP) { - packet_sent = true; - /*Put packet to retransmission queue*/ - eigrp_fifo_push(nbr->retrans_queue, ep); + struct eigrp_packet *ep_dup; - if (nbr->retrans_queue->count == 1) { - eigrp_send_packet_reliably(nbr); - } + if (nbr->state != EIGRP_NEIGHBOR_UP) + continue; + + if (packet_sent) + ep_dup = eigrp_packet_duplicate(ep); + else + ep_dup = ep; + + packet_sent = true; + /*Put packet to retransmission queue*/ + eigrp_fifo_push(nbr->retrans_queue, ep_dup); + + if (nbr->retrans_queue->count == 1) { + eigrp_send_packet_reliably(nbr); } } From a6e8810ee341b0c1fc959ca0c9ed044990b4bfc8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 19 Aug 2017 14:15:34 -0400 Subject: [PATCH 2/3] eigrpd: Allow eigrp_update_send to recognize a full packet Modify code to allow the eigrp_update_send function to recognize that we have a full packet and to do the right thing from there. Signed-off-by: Donald Sharp --- eigrpd/eigrp_update.c | 90 +++++++++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 25 deletions(-) diff --git a/eigrpd/eigrp_update.c b/eigrpd/eigrp_update.c index e42f84abc6..8971635bfd 100644 --- a/eigrpd/eigrp_update.c +++ b/eigrpd/eigrp_update.c @@ -534,6 +534,38 @@ static void eigrp_update_place_on_nbr_queue(struct eigrp_neighbor *nbr, eigrp_send_packet_reliably(nbr); } +static void eigrp_update_send_to_all_nbrs(struct eigrp_interface *ei, + struct eigrp_packet *ep) +{ + struct listnode *node, *nnode; + struct eigrp_neighbor *nbr; + bool packet_sent = false; + + for (ALL_LIST_ELEMENTS(ei->nbrs, node, nnode, nbr)) { + struct eigrp_packet *ep_dup; + + if (nbr->state != EIGRP_NEIGHBOR_UP) + continue; + + if (packet_sent) + ep_dup = eigrp_packet_duplicate(ep, NULL); + else + ep_dup = ep; + + ep_dup->nbr = nbr; + packet_sent = true; + /*Put packet to retransmission queue*/ + eigrp_fifo_push(nbr->retrans_queue, ep_dup); + + if (nbr->retrans_queue->count == 1) { + eigrp_send_packet_reliably(nbr); + } + } + + if (!packet_sent) + eigrp_packet_free(ep); +} + void eigrp_update_send_EOT(struct eigrp_neighbor *nbr) { struct eigrp_packet *ep; @@ -617,13 +649,13 @@ void eigrp_update_send_EOT(struct eigrp_neighbor *nbr) } eigrp_update_place_on_nbr_queue (nbr, ep, seq_no, length); + nbr->ei->eigrp->sequence_number = seq_no++; } void eigrp_update_send(struct eigrp_interface *ei) { struct eigrp_packet *ep; struct listnode *node, *nnode; - struct eigrp_neighbor *nbr; struct eigrp_prefix_entry *pe; u_char has_tlv; struct access_list *alist; @@ -632,7 +664,10 @@ void eigrp_update_send(struct eigrp_interface *ei) struct prefix_list *plist_i; struct eigrp *e; struct prefix_ipv4 *dest_addr; - bool packet_sent = false; + u_int32_t seq_no = ei->eigrp->sequence_number; + + if (ei->nbrs->count == 0) + return; u_int16_t length = EIGRP_HEADER_LEN; @@ -640,7 +675,7 @@ void eigrp_update_send(struct eigrp_interface *ei) /* Prepare EIGRP INIT UPDATE header */ eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei, ep->s, 0, - ei->eigrp->sequence_number, 0); + seq_no, 0); // encode Authentication TLV, if needed if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) @@ -655,6 +690,31 @@ void eigrp_update_send(struct eigrp_interface *ei) if (!(pe->req_action & EIGRP_FSM_NEED_UPDATE)) continue; + if ((length + 0x001D) > (u_int16_t)ei->ifp->mtu) { + if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) + && (IF_DEF_PARAMS(ei->ifp)->auth_keychain != NULL)) { + eigrp_make_md5_digest(ei, ep->s, EIGRP_AUTH_UPDATE_FLAG); + } + + eigrp_packet_checksum(ei, ep->s, length); + ep->length = length; + + ep->dst.s_addr = htonl(EIGRP_MULTICAST_ADDRESS); + + ep->sequence_number = seq_no; + seq_no++; + eigrp_update_send_to_all_nbrs(ei, ep); + + length = EIGRP_HEADER_LEN; + ep = eigrp_packet_new(ei->ifp->mtu, NULL); + eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei, ep->s, 0, + seq_no, 0); + if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) + && (IF_DEF_PARAMS(ei->ifp)->auth_keychain != NULL)) { + length += eigrp_add_authTLV_MD5_to_stream(ep->s, ei); + } + has_tlv = 0; + } /* Get destination address from prefix */ dest_addr = pe->destination_ipv4; @@ -718,28 +778,8 @@ void eigrp_update_send(struct eigrp_interface *ei) zlog_debug("Enqueuing Update length[%u] Seq [%u]", length, ep->sequence_number); - for (ALL_LIST_ELEMENTS(ei->nbrs, node, nnode, nbr)) { - struct eigrp_packet *ep_dup; - - if (nbr->state != EIGRP_NEIGHBOR_UP) - continue; - - if (packet_sent) - ep_dup = eigrp_packet_duplicate(ep); - else - ep_dup = ep; - - packet_sent = true; - /*Put packet to retransmission queue*/ - eigrp_fifo_push(nbr->retrans_queue, ep_dup); - - if (nbr->retrans_queue->count == 1) { - eigrp_send_packet_reliably(nbr); - } - } - - if (!packet_sent) - eigrp_packet_free(ep); + eigrp_update_send_to_all_nbrs(ei, ep); + ei->eigrp->sequence_number = seq_no++; } void eigrp_update_send_all(struct eigrp *eigrp, From cf2f4daee37d1cbb92d29cbb3f0bf3268ee372cd Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 19 Aug 2017 14:38:17 -0400 Subject: [PATCH 3/3] eigrpd: Pass in actual used parameter to header creation Signed-off-by: Donald Sharp --- eigrpd/eigrp_hello.c | 2 +- eigrpd/eigrp_packet.c | 6 +++--- eigrpd/eigrp_packet.h | 2 +- eigrpd/eigrp_query.c | 2 +- eigrpd/eigrp_reply.c | 2 +- eigrpd/eigrp_siaquery.c | 2 +- eigrpd/eigrp_siareply.c | 2 +- eigrpd/eigrp_update.c | 26 ++++++++++++++------------ 8 files changed, 23 insertions(+), 21 deletions(-) diff --git a/eigrpd/eigrp_hello.c b/eigrpd/eigrp_hello.c index 67f75c536b..d56767fafc 100644 --- a/eigrpd/eigrp_hello.c +++ b/eigrpd/eigrp_hello.c @@ -618,7 +618,7 @@ static struct eigrp_packet *eigrp_hello_encode(struct eigrp_interface *ei, if (ep) { // encode common header feilds - eigrp_packet_header_init(EIGRP_OPC_HELLO, ei, ep->s, 0, 0, ack); + eigrp_packet_header_init(EIGRP_OPC_HELLO, ei->eigrp, ep->s, 0, 0, ack); // encode Authentication TLV if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) diff --git a/eigrpd/eigrp_packet.c b/eigrpd/eigrp_packet.c index dde553d53f..3b4a57ed54 100644 --- a/eigrpd/eigrp_packet.c +++ b/eigrpd/eigrp_packet.c @@ -877,7 +877,7 @@ void eigrp_packet_checksum(struct eigrp_interface *ei, struct stream *s, } /* Make EIGRP header. */ -void eigrp_packet_header_init(int type, struct eigrp_interface *ei, +void eigrp_packet_header_init(int type, struct eigrp *eigrp, struct stream *s, u_int32_t flags, u_int32_t sequence, u_int32_t ack) { @@ -890,8 +890,8 @@ void eigrp_packet_header_init(int type, struct eigrp_interface *ei, eigrph->opcode = (u_char)type; eigrph->checksum = 0; - eigrph->vrid = htons(ei->eigrp->vrid); - eigrph->ASNumber = htons(ei->eigrp->AS); + eigrph->vrid = htons(eigrp->vrid); + eigrph->ASNumber = htons(eigrp->AS); eigrph->ack = htonl(ack); eigrph->sequence = htonl(sequence); // if(flags == EIGRP_INIT_FLAG) diff --git a/eigrpd/eigrp_packet.h b/eigrpd/eigrp_packet.h index 040204a7fc..03fe412f1f 100644 --- a/eigrpd/eigrp_packet.h +++ b/eigrpd/eigrp_packet.h @@ -41,7 +41,7 @@ extern struct eigrp_packet *eigrp_packet_duplicate(struct eigrp_packet *, struct eigrp_neighbor *); extern void eigrp_packet_free(struct eigrp_packet *); extern void eigrp_packet_delete(struct eigrp_interface *); -extern void eigrp_packet_header_init(int, struct eigrp_interface *, +extern void eigrp_packet_header_init(int, struct eigrp *, struct stream *, u_int32_t, u_int32_t, u_int32_t); extern void eigrp_packet_checksum(struct eigrp_interface *, struct stream *, diff --git a/eigrpd/eigrp_query.c b/eigrpd/eigrp_query.c index d6299ad923..3bca444ab7 100644 --- a/eigrpd/eigrp_query.c +++ b/eigrpd/eigrp_query.c @@ -162,7 +162,7 @@ void eigrp_send_query(struct eigrp_interface *ei) ep = eigrp_packet_new(ei->ifp->mtu, NULL); /* Prepare EIGRP INIT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_QUERY, ei, ep->s, 0, + eigrp_packet_header_init(EIGRP_OPC_QUERY, ei->eigrp, ep->s, 0, ei->eigrp->sequence_number, 0); // encode Authentication TLV, if needed diff --git a/eigrpd/eigrp_reply.c b/eigrpd/eigrp_reply.c index 60390ad8a3..e59db80393 100644 --- a/eigrpd/eigrp_reply.c +++ b/eigrpd/eigrp_reply.c @@ -113,7 +113,7 @@ void eigrp_send_reply(struct eigrp_neighbor *nbr, struct eigrp_prefix_entry *pe) ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); /* Prepare EIGRP INIT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_REPLY, nbr->ei, ep->s, 0, + eigrp_packet_header_init(EIGRP_OPC_REPLY, e, ep->s, 0, nbr->ei->eigrp->sequence_number, 0); // encode Authentication TLV, if needed diff --git a/eigrpd/eigrp_siaquery.c b/eigrpd/eigrp_siaquery.c index 215df7b8ee..30f65ee87d 100644 --- a/eigrpd/eigrp_siaquery.c +++ b/eigrpd/eigrp_siaquery.c @@ -126,7 +126,7 @@ void eigrp_send_siaquery(struct eigrp_neighbor *nbr, ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); /* Prepare EIGRP INIT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_SIAQUERY, nbr->ei, ep->s, 0, + eigrp_packet_header_init(EIGRP_OPC_SIAQUERY, nbr->ei->eigrp, ep->s, 0, nbr->ei->eigrp->sequence_number, 0); // encode Authentication TLV, if needed diff --git a/eigrpd/eigrp_siareply.c b/eigrpd/eigrp_siareply.c index 32f0c8be33..3050b91032 100644 --- a/eigrpd/eigrp_siareply.c +++ b/eigrpd/eigrp_siareply.c @@ -125,7 +125,7 @@ void eigrp_send_siareply(struct eigrp_neighbor *nbr, ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); /* Prepare EIGRP INIT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_SIAREPLY, nbr->ei, ep->s, 0, + eigrp_packet_header_init(EIGRP_OPC_SIAREPLY, nbr->ei->eigrp, ep->s, 0, nbr->ei->eigrp->sequence_number, 0); // encode Authentication TLV, if needed diff --git a/eigrpd/eigrp_update.c b/eigrpd/eigrp_update.c index 8971635bfd..7a7b1dd5d6 100644 --- a/eigrpd/eigrp_update.c +++ b/eigrpd/eigrp_update.c @@ -471,9 +471,10 @@ void eigrp_update_send_init(struct eigrp_neighbor *nbr) nbr->ei->eigrp->sequence_number, nbr->recv_sequence_number); - eigrp_packet_header_init( - EIGRP_OPC_UPDATE, nbr->ei, ep->s, EIGRP_INIT_FLAG, - nbr->ei->eigrp->sequence_number, nbr->recv_sequence_number); + eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei->eigrp, + ep->s, EIGRP_INIT_FLAG, + nbr->ei->eigrp->sequence_number, + nbr->recv_sequence_number); // encode Authentication TLV, if needed if ((IF_DEF_PARAMS(nbr->ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) @@ -584,9 +585,9 @@ void eigrp_update_send_EOT(struct eigrp_neighbor *nbr) ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); /* Prepare EIGRP EOT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei, ep->s, EIGRP_EOT_FLAG, - seq_no, - nbr->recv_sequence_number); + eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei->eigrp, + ep->s, EIGRP_EOT_FLAG, + seq_no, nbr->recv_sequence_number); // encode Authentication TLV, if needed if((IF_DEF_PARAMS (nbr->ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) && @@ -606,7 +607,8 @@ void eigrp_update_send_EOT(struct eigrp_neighbor *nbr) length = EIGRP_HEADER_LEN; ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); - eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei, ep->s, EIGRP_EOT_FLAG, + eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei->eigrp, + ep->s, EIGRP_EOT_FLAG, seq_no, nbr->recv_sequence_number); if((IF_DEF_PARAMS (nbr->ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) && @@ -674,8 +676,8 @@ void eigrp_update_send(struct eigrp_interface *ei) ep = eigrp_packet_new(ei->ifp->mtu, NULL); /* Prepare EIGRP INIT UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei, ep->s, 0, - seq_no, 0); + eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei->eigrp, + ep->s, 0, seq_no, 0); // encode Authentication TLV, if needed if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) @@ -707,8 +709,8 @@ void eigrp_update_send(struct eigrp_interface *ei) length = EIGRP_HEADER_LEN; ep = eigrp_packet_new(ei->ifp->mtu, NULL); - eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei, ep->s, 0, - seq_no, 0); + eigrp_packet_header_init(EIGRP_OPC_UPDATE, ei->eigrp, + ep->s, 0, seq_no, 0); if ((IF_DEF_PARAMS(ei->ifp)->auth_type == EIGRP_AUTH_TYPE_MD5) && (IF_DEF_PARAMS(ei->ifp)->auth_keychain != NULL)) { length += eigrp_add_authTLV_MD5_to_stream(ep->s, ei); @@ -875,7 +877,7 @@ static void eigrp_update_send_GR_part(struct eigrp_neighbor *nbr) ep = eigrp_packet_new(nbr->ei->ifp->mtu, nbr); /* Prepare EIGRP Graceful restart UPDATE header */ - eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei, ep->s, flags, + eigrp_packet_header_init(EIGRP_OPC_UPDATE, nbr->ei->eigrp, ep->s, flags, nbr->ei->eigrp->sequence_number, nbr->recv_sequence_number);