summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPooja Jagadeesh Doijode <pdoijode@nvidia.com>2023-04-18 18:40:06 -0700
committerPooja Jagadeesh Doijode <pdoijode@nvidia.com>2023-04-18 18:40:06 -0700
commit6e076ba5231cba2e22dcbdc48a9c13df046a2e47 (patch)
treecbdf98d92916f6e6eea689d9adf96dd8923521bc
parent74aad5ab559cc39f52137292b3b38d2d07eff428 (diff)
bgpd: Fix for ain->attr corruption during path update
1. Consider a established L2VPN EVPN BGP peer with soft-reconfiguartion inbound configured 2. When the interface of this directly connected BGP peer is shutdown, bgp_soft_reconfig_table_update() is called, which memsets the evpn buffer and calls bgp_update() with received attributes stored in ain table(ain->attr). In bgp_update(), evpn_overlay attribute in ain->attr (which is an interned attr) was modified by doing a memcpy 3. Above action causes 2 attributes in the attrhash (which were previously different) to match! 4. Later during fsm change event of the peer, bgp_adj_in_remove() is called to clean up the ain->attr. But, because 2 attrs in attrhash match, it causes BGP to assert in bgp_attr_unintern() Signed-off-by: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
-rw-r--r--bgpd/bgp_route.c30
1 files changed, 23 insertions, 7 deletions
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 65f7ce1643..7809d9b0a9 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4065,17 +4065,24 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
if (has_valid_label)
assert(label != NULL);
- /* Update overlay index of the attribute */
- if (afi == AFI_L2VPN && evpn)
- memcpy(&attr->evpn_overlay, evpn,
- sizeof(struct bgp_route_evpn));
/* When peer's soft reconfiguration enabled. Record input packet in
Adj-RIBs-In. */
- if (!soft_reconfig
- && CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG)
- && peer != bgp->peer_self)
+ if (!soft_reconfig &&
+ CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG) &&
+ peer != bgp->peer_self) {
+ /*
+ * If the trigger is not from soft_reconfig and if
+ * PEER_FLAG_SOFT_RECONFIG is enabled for the peer, then attr
+ * will not be interned. In which case, it is ok to update the
+ * attr->evpn_overlay, so that, this can be stored in adj_in.
+ */
+ if ((afi == AFI_L2VPN) && evpn) {
+ memcpy(&attr->evpn_overlay, evpn,
+ sizeof(struct bgp_route_evpn));
+ }
bgp_adj_in_set(dest, peer, attr, addpath_id);
+ }
/* Update permitted loop count */
if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_ALLOWAS_IN))
@@ -4203,6 +4210,15 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
}
new_attr = *attr;
+ /*
+ * If bgp_update is called with soft_reconfig set then
+ * attr is interned. In this case, do not overwrite the
+ * attr->evpn_overlay with evpn directly. Instead memcpy
+ * evpn to new_atr.evpn_overlay before it is interned.
+ */
+ if (soft_reconfig && (afi == AFI_L2VPN) && evpn)
+ memcpy(&new_attr.evpn_overlay, evpn,
+ sizeof(struct bgp_route_evpn));
/* Apply incoming route-map.
* NB: new_attr may now contain newly allocated values from route-map