From b3b3a910a693b2955a22bdd624ac66e6ead90c08 Mon Sep 17 00:00:00 2001 From: anlan_cs Date: Mon, 6 Feb 2023 09:27:05 +0800 Subject: [PATCH] bgpd: fix use-after-free crash for evpn ``` anlan(config-router-af)# vni 33 anlan(config-router-af-vni)# route-target both 44:55 anlan(config-router-af-vni)# no route-target both 44:55 vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error! ``` When `bgp_evpn_vni_rt_cmd` deals with "both" type, it wrongly created only one node ( should be two nodes ) for lists of both `vpn->import_rtl` and `vpn->export_rtl`. At this time, the two lists are already wrong. In `no route-target both RT`, it will free the single node from lists of both `vpn->import_rtl` and `vpn->export_rtl`. After freed from `vpn->import_rtl`, it is "use-after-free" at the time of freeing it from `vpn->export_rtl`. It causes crash sometimes, or other unexpected behaviours. This issue is introduced by commit `3b7e8d`, which have adjusted both `bgp_evpn_vni_rt_cmd` and `bgp_evpn_vrf_rt_cmd`. Since `bgp_evpn_vrf_rt_cmd/no_bgp_evpn_vrf_rt_cmd` works well again unintentionally with commit `7022da`, only `bgp_evpn_vni_rt_cmd` needs to modify - add two nodes for "both" type and some explicit comments for this special case of "both" type. Signed-off-by: anlan_cs (cherry picked from commit 432ff4b036860fb626f3027a7798038d594b8042) --- bgpd/bgp_evpn_vty.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index 0b5f81ccb0..6b63c6e3aa 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -6896,15 +6896,17 @@ DEFUN (bgp_evpn_vni_rt, return CMD_WARNING; } - ecomadd = ecommunity_str2com(argv[2]->arg, ECOMMUNITY_ROUTE_TARGET, 0); - if (!ecomadd) { - vty_out(vty, "%% Malformed Route Target list\n"); - return CMD_WARNING; - } - ecommunity_str(ecomadd); - /* Add/update the import route-target */ if (rt_type == RT_TYPE_BOTH || rt_type == RT_TYPE_IMPORT) { + /* Note that first of the two RTs is created for "both" type */ + ecomadd = ecommunity_str2com(argv[2]->arg, + ECOMMUNITY_ROUTE_TARGET, 0); + if (!ecomadd) { + vty_out(vty, "%% Malformed Route Target list\n"); + return CMD_WARNING; + } + ecommunity_str(ecomadd); + /* Do nothing if we already have this import route-target */ if (!bgp_evpn_rt_matches_existing(vpn->import_rtl, ecomadd)) evpn_configure_import_rt(bgp, vpn, ecomadd); @@ -6912,6 +6914,15 @@ DEFUN (bgp_evpn_vni_rt, /* Add/update the export route-target */ if (rt_type == RT_TYPE_BOTH || rt_type == RT_TYPE_EXPORT) { + /* Note that second of the two RTs is created for "both" type */ + ecomadd = ecommunity_str2com(argv[2]->arg, + ECOMMUNITY_ROUTE_TARGET, 0); + if (!ecomadd) { + vty_out(vty, "%% Malformed Route Target list\n"); + return CMD_WARNING; + } + ecommunity_str(ecomadd); + /* Do nothing if we already have this export route-target */ if (!bgp_evpn_rt_matches_existing(vpn->export_rtl, ecomadd)) evpn_configure_export_rt(bgp, vpn, ecomadd); -- 2.39.5