From 3bd70bf8f37bd899b82534d71572714404eb94c9 Mon Sep 17 00:00:00 2001 From: "G. Paul Ziemba" Date: Fri, 29 Jun 2018 14:55:59 -0700 Subject: [PATCH] bgpd: fix NULL dereference in vrf-vpn leak config if before default instance Issue 2473 VRF-VPN route-leaking configuration commands assumed existence of default bgp instance. If a non-default vrf configuration occurred before the default vrf configuration, it would result in a (NULL) dereference to the non-existent default vrf. This change 1) detects non-existence of default vrf and skips corresponding RIB updating that normally occurs during configuration changes of the route-leaking parameters; and 2) when the default bgp instance is defined, runs the deferred RIB updating. In vpn_leak_to_vrf_update_all(), replace early return if bgp_vpn is NULL with assert, as the former would lead to subtly wrong RIB contents. The underlying issue that led to commit 73aed5841a1fe31e8a1f251dfcf87e1f2b1f2463 should be fixed by the above changes. Signed-off-by: G. Paul Ziemba --- bgpd/bgp_mplsvpn.c | 68 ++++++++++++++++++++++++++++++++++++++++++++-- bgpd/bgp_mplsvpn.h | 10 +++++++ bgpd/bgp_vty.c | 8 ++++++ 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 3a854be534..db2acf8058 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -490,7 +490,7 @@ leak_update( * (only one hop back to ultimate parent for vrf-vpn-vrf scheme). * Using a loop here supports more complex intra-bgp import-export * schemes that could be implemented in the future. - * + * */ for (bi_ultimate = source_bi; bi_ultimate->extra && bi_ultimate->extra->parent; @@ -1356,8 +1356,7 @@ void vpn_leak_to_vrf_update_all(struct bgp *bgp_vrf, /* to */ struct bgp_node *prn; safi_t safi = SAFI_MPLS_VPN; - if (!bgp_vpn) - return; + assert(bgp_vpn); /* * Walk vpn table @@ -2253,3 +2252,66 @@ vrf_id_t get_first_vrf_for_redirect_with_rt(struct ecommunity *eckey) } return VRF_UNKNOWN; } + +/* + * The purpose of this function is to process leaks that were deferred + * from earlier per-vrf configuration due to not-yet-existing default + * vrf, in other words, configuration such as: + * + * router bgp MMM vrf FOO + * address-family ipv4 unicast + * rd vpn export 1:1 + * exit-address-family + * + * router bgp NNN + * ... + * + * This function gets called when the default instance ("router bgp NNN") + * is created. + */ +void vpn_leak_postchange_all(void) +{ + struct listnode *next; + struct bgp *bgp; + struct bgp *bgp_default = bgp_get_default(); + + assert(bgp_default); + + /* First, do any exporting from VRFs to the single VPN RIB */ + for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, bgp)) { + + if (bgp->inst_type != BGP_INSTANCE_TYPE_VRF) + continue; + + vpn_leak_postchange( + BGP_VPN_POLICY_DIR_TOVPN, + AFI_IP, + bgp_default, + bgp); + + vpn_leak_postchange( + BGP_VPN_POLICY_DIR_TOVPN, + AFI_IP6, + bgp_default, + bgp); + } + + /* Now, do any importing to VRFs from the single VPN RIB */ + for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, bgp)) { + + if (bgp->inst_type != BGP_INSTANCE_TYPE_VRF) + continue; + + vpn_leak_postchange( + BGP_VPN_POLICY_DIR_FROMVPN, + AFI_IP, + bgp_default, + bgp); + + vpn_leak_postchange( + BGP_VPN_POLICY_DIR_FROMVPN, + AFI_IP6, + bgp_default, + bgp); + } +} diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 384108dc0c..b0add40da9 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -182,6 +182,10 @@ static inline void vpn_leak_prechange(vpn_policy_direction_t direction, afi_t afi, struct bgp *bgp_vpn, struct bgp *bgp_vrf) { + /* Detect when default bgp instance is not (yet) defined by config */ + if (!bgp_vpn) + return; + if ((direction == BGP_VPN_POLICY_DIR_FROMVPN) && vpn_leak_from_vpn_active(bgp_vrf, afi, NULL)) { @@ -198,6 +202,10 @@ static inline void vpn_leak_postchange(vpn_policy_direction_t direction, afi_t afi, struct bgp *bgp_vpn, struct bgp *bgp_vrf) { + /* Detect when default bgp instance is not (yet) defined by config */ + if (!bgp_vpn) + return; + if (direction == BGP_VPN_POLICY_DIR_FROMVPN) vpn_leak_to_vrf_update_all(bgp_vrf, bgp_vpn, afi); if (direction == BGP_VPN_POLICY_DIR_TOVPN) { @@ -216,4 +224,6 @@ extern void vpn_policy_routemap_event(const char *rmap_name); extern vrf_id_t get_first_vrf_for_redirect_with_rt(struct ecommunity *eckey); +extern void vpn_leak_postchange_all(void); + #endif /* _QUAGGA_BGP_MPLSVPN_H */ diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 86f3f97c49..88c310d9d5 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -928,6 +928,14 @@ DEFUN_NOSH (router_bgp, return CMD_WARNING_CONFIG_FAILED; } + /* + * If we just instantiated the default instance, complete + * any pending VRF-VPN leaking that was configured via + * earlier "router bgp X vrf FOO" blocks. + */ + if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) + vpn_leak_postchange_all(); + /* Pending: handle when user tries to change a view to vrf n vv. */ } -- 2.39.5