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>
(cherry picked from commit 6e076ba5231cba2e22dcbdc48a9c13df046a2e47)

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 493d9e8a6e326ff633aa3b3312142482385f8e73..a57a6b484e5dbe5aa96fb352072b771d1eb964dc 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3910,17 +3910,24 @@ int 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);
+       }
 
        /* Check previously received route. */
        for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next)
@@ -4030,6 +4037,15 @@ int 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