diff options
| author | Pooja Jagadeesh Doijode <pdoijode@nvidia.com> | 2023-04-18 18:40:06 -0700 |
|---|---|---|
| committer | Pooja Jagadeesh Doijode <pdoijode@nvidia.com> | 2023-04-18 18:40:06 -0700 |
| commit | 6e076ba5231cba2e22dcbdc48a9c13df046a2e47 (patch) | |
| tree | cbdf98d92916f6e6eea689d9adf96dd8923521bc | |
| parent | 74aad5ab559cc39f52137292b3b38d2d07eff428 (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.c | 30 |
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 |
