]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: fix use-after-free crash for evpn
authoranlan_cs <vic.lan@pica8.com>
Mon, 6 Feb 2023 01:27:05 +0000 (09:27 +0800)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Wed, 8 Feb 2023 08:09:37 +0000 (08:09 +0000)
```
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 <vic.lan@pica8.com>
(cherry picked from commit 432ff4b036860fb626f3027a7798038d594b8042)

bgpd/bgp_evpn_vty.c

index 5ad5cf8bff3b78c9ac01bd3cafa72c240d69e02f..ac5f2cfa133404ee11e934f7783828445070909b 100644 (file)
@@ -6054,15 +6054,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);
@@ -6070,6 +6072,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);