From 96885f16b0838d3324db86153d50e27fe2cef3a6 Mon Sep 17 00:00:00 2001 From: vivek Date: Fri, 22 Jan 2016 10:56:48 -0800 Subject: [PATCH] BGP: Rework iteration of peer_af_array While processing references to the macro PEERAF_FOREACH(), aggressive loop optimization by gcc 4.9.x (probably 4.8 and greater) was resulting in the generated code not checking on the index as well as eliminating some code. This was leading to a dereference of invalid memory when a BGP peer came up. The fix is to scrap this convoluted macro. Two other changes done are to eliminate overloading of "afindex" and make the loop iterator an integer. Signed-off-by: Vivek Venkatraman Reviewed-by: Dave Olson Reviewed-by: Donald Sharp Reviewed-by: Daniel Walton Ticket: CM-8889 Reviewed By: CCR-4018 Testing Done: Verified failure scenario Note: This code was added as part of update-groups implementation; when upstreaming update-groups, this patch should also be included. --- bgpd/bgp_debug.c | 24 ++++++++++++++++-------- bgpd/bgp_updgrp.h | 28 +++++++++++++++++++++------- bgpd/bgpd.c | 10 +++++++--- bgpd/bgpd.h | 6 ------ 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c index 68106b449e..c86df535aa 100644 --- a/bgpd/bgp_debug.c +++ b/bgpd/bgp_debug.c @@ -1081,18 +1081,22 @@ DEFUN (debug_bgp_update_direct_peer, { struct peer *peer; struct peer_af *paf; - enum bgp_af_index af; + int afidx; bgp_debug_list_add_entry(bgp_debug_update_out_peers, host, NULL); peer = bgp_find_peer (vty, host); if (peer) { - PEERAF_FOREACH (peer, paf, af) + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) { - if (PAF_SUBGRP (paf)) + paf = peer->peer_af_array[afidx]; + if (paf != NULL) { - UPDGRP_PEER_DBG_EN(PAF_SUBGRP(paf)->update_group); + if (PAF_SUBGRP (paf)) + { + UPDGRP_PEER_DBG_EN(PAF_SUBGRP(paf)->update_group); + } } } } @@ -1220,16 +1224,20 @@ DEFUN (no_debug_bgp_update_direct_peer, struct peer *peer; struct peer_af *paf; - enum bgp_af_index af; + int afidx; peer = bgp_find_peer (vty, host); if (peer) { - PEERAF_FOREACH (peer, paf, af) + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) { - if (PAF_SUBGRP (paf)) + paf = peer->peer_af_array[afidx]; + if (paf != NULL) { - UPDGRP_PEER_DBG_DIS(PAF_SUBGRP(paf)->update_group); + if (PAF_SUBGRP (paf)) + { + UPDGRP_PEER_DBG_DIS(PAF_SUBGRP(paf)->update_group); + } } } } diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h index b94519ef00..aab2458b30 100644 --- a/bgpd/bgp_updgrp.h +++ b/bgpd/bgp_updgrp.h @@ -528,9 +528,14 @@ static inline void update_group_adjust_peer_afs (struct peer *peer) { struct peer_af *paf; - enum bgp_af_index afi; + int afidx; - PEERAF_FOREACH (peer, paf, afi) update_group_adjust_peer (paf); + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) + { + paf = peer->peer_af_array[afidx]; + if (paf != NULL) + update_group_adjust_peer (paf); + } } /* @@ -542,10 +547,14 @@ static inline void update_group_remove_peer_afs (struct peer *peer) { struct peer_af *paf; - enum bgp_af_index afi; + int afidx; - PEERAF_FOREACH (peer, paf, afi) - update_subgroup_remove_peer (PAF_SUBGRP (paf), paf); + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) + { + paf = peer->peer_af_array[afidx]; + if (paf != NULL) + update_subgroup_remove_peer (PAF_SUBGRP (paf), paf); + } } /* @@ -592,9 +601,14 @@ static inline void bgp_announce_peer (struct peer *peer) { struct peer_af *paf; - enum bgp_af_index af; + int afidx; - PEERAF_FOREACH (peer, paf, af) subgroup_announce_all (PAF_SUBGRP (paf)); + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) + { + paf = peer->peer_af_array[afidx]; + if (paf != NULL) + subgroup_announce_all (PAF_SUBGRP (paf)); + } } /** diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index af2f284dab..c7f9232a89 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1142,7 +1142,7 @@ peer_xfer_config (struct peer *peer_dst, struct peer *peer_src) struct peer_af *paf; afi_t afi; safi_t safi; - enum bgp_af_index afindex; + int afidx; assert(peer_src); assert(peer_dst); @@ -1185,8 +1185,12 @@ peer_xfer_config (struct peer *peer_dst, struct peer *peer_src) peer_dst->allowas_in[afi][safi] = peer_src->allowas_in[afi][safi]; } - PEERAF_FOREACH(peer_src, paf, afindex) - peer_af_create(peer_dst, paf->afi, paf->safi); + for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) + { + paf = peer_src->peer_af_array[afidx]; + if (paf != NULL) + peer_af_create(peer_dst, paf->afi, paf->safi); + } /* update-source apply */ if (peer_src->update_source) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 14ebf840ab..7af51f304c 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -840,12 +840,6 @@ struct bgp_nlri bgp_size_t length; }; -#define PEERAF_FOREACH(peer, paf, afi) \ - for ((afi) = BGP_AF_START, (paf) = (peer)->peer_af_array[(afi)]; \ - (afi) < BGP_AF_MAX; \ - (afi)++, (paf) = (peer)->peer_af_array[(afi)]) \ - if ((paf) != NULL) \ - /* BGP versions. */ #define BGP_VERSION_4 4 -- 2.39.5