]> 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)
committeranlan_cs <vic.lan@pica8.com>
Mon, 6 Feb 2023 13:39:22 +0000 (21:39 +0800)
```
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>
bgpd/bgp_evpn_vty.c

index 0b5f81ccb006a48259242c6ebe3240b018367141..6b63c6e3aa276765850e9156ef327e2e9d60c159 100644 (file)
@@ -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);