]> git.puffer.fish Git - mirror/frr.git/commitdiff
zebra: fix use of freed es during zebra shutdown 7164/head
authorAnuradha Karuppiah <anuradhak@cumulusnetworks.com>
Tue, 15 Sep 2020 23:50:14 +0000 (16:50 -0700)
committerAnuradha Karuppiah <anuradhak@cumulusnetworks.com>
Wed, 23 Sep 2020 18:20:13 +0000 (11:20 -0700)
This problem was reported by the sanitizer -
=================================================================
==24764==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000115c8 at pc 0x55cb9cfad312 bp 0x7fffa0552140 sp 0x7fffa0552138
READ of size 8 at 0x60d0000115c8 thread T0
    #0 0x55cb9cfad311 in zebra_evpn_remote_es_flush zebra/zebra_evpn_mh.c:2041
    #1 0x55cb9cfad311 in zebra_evpn_es_cleanup zebra/zebra_evpn_mh.c:2234
    #2 0x55cb9cf6ae78 in zebra_vrf_disable zebra/zebra_vrf.c:205
    #3 0x7fc8d478f114 in vrf_delete lib/vrf.c:229
    #4 0x7fc8d478f99a in vrf_terminate lib/vrf.c:541
    #5 0x55cb9ceba0af in sigint zebra/main.c:176
    #6 0x55cb9ceba0af in sigint zebra/main.c:130
    #7 0x7fc8d4765d20 in quagga_sigevent_process lib/sigevent.c:103
    #8 0x7fc8d4787e8c in thread_fetch lib/thread.c:1396
    #9 0x7fc8d4708782 in frr_run lib/libfrr.c:1092
    #10 0x55cb9ce931d8 in main zebra/main.c:488
    #11 0x7fc8d43ee09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #12 0x55cb9ce94c09 in _start (/usr/lib/frr/zebra+0x8ac09)
=================================================================

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
zebra/zebra_evpn_mh.c

index 7634b1255a84cd3fb87154b69d4eeda4b73a0cf5..42710a33db455c4ccb7918a8910a29c24e68fe0b 100644 (file)
@@ -60,7 +60,7 @@ DEFINE_MTYPE_STATIC(ZEBRA, ZES_VTEP, "VTEP attached to the ES");
 static void zebra_evpn_es_get_one_base_evpn(void);
 static int zebra_evpn_es_evi_send_to_client(struct zebra_evpn_es *es,
                zebra_evpn_t *zevpn, bool add);
-static void zebra_evpn_local_es_del(struct zebra_evpn_es *es);
+static void zebra_evpn_local_es_del(struct zebra_evpn_es **esp);
 static int zebra_evpn_local_es_update(struct zebra_if *zif, uint32_t lid,
                struct ethaddr *sysmac);
 
@@ -820,7 +820,7 @@ void zebra_evpn_if_cleanup(struct zebra_if *zif)
 
        /* Delete associated Ethernet Segment */
        if (zif->es_info.es)
-               zebra_evpn_local_es_del(zif->es_info.es);
+               zebra_evpn_local_es_del(&zif->es_info.es);
 }
 
 /*****************************************************************************
@@ -1107,15 +1107,17 @@ static struct zebra_evpn_es *zebra_evpn_es_new(esi_t *esi)
  * This just frees appropriate memory, caller should have taken other
  * needed actions.
  */
-static struct zebra_evpn_es *zebra_evpn_es_free(struct zebra_evpn_es *es)
+static void zebra_evpn_es_free(struct zebra_evpn_es **esp)
 {
+       struct zebra_evpn_es *es = *esp;
+
        /* If the ES has a local or remote reference it cannot be freed.
         * Free is also prevented if there are MAC entries referencing
         * it.
         */
        if ((es->flags & (ZEBRA_EVPNES_LOCAL | ZEBRA_EVPNES_REMOTE)) ||
                        listcount(es->mac_list))
-               return es;
+               return;
 
        if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
                zlog_debug("es %s free", es->esi_str);
@@ -1137,7 +1139,7 @@ static struct zebra_evpn_es *zebra_evpn_es_free(struct zebra_evpn_es *es)
 
        XFREE(MTYPE_ZES, es);
 
-       return NULL;
+       *esp = NULL;
 }
 
 /* Inform BGP about local ES addition */
@@ -1348,9 +1350,10 @@ static void zebra_evpn_es_local_info_set(struct zebra_evpn_es *es,
                        false /* force_clear_static */);
 }
 
-static void zebra_evpn_es_local_info_clear(struct zebra_evpn_es *es)
+static void zebra_evpn_es_local_info_clear(struct zebra_evpn_es **esp)
 {
        struct zebra_if *zif;
+       struct zebra_evpn_es *es = *esp;
 
        if (!(es->flags & ZEBRA_EVPNES_LOCAL))
                return;
@@ -1372,16 +1375,17 @@ static void zebra_evpn_es_local_info_clear(struct zebra_evpn_es *es)
        list_delete_node(zmh_info->local_es_list, &es->local_es_listnode);
 
        /* free up the ES if there is no remote reference */
-       zebra_evpn_es_free(es);
+       zebra_evpn_es_free(esp);
 }
 
 /* Delete an ethernet segment and inform BGP */
-static void zebra_evpn_local_es_del(struct zebra_evpn_es *es)
+static void zebra_evpn_local_es_del(struct zebra_evpn_es **esp)
 {
        struct zebra_evpn_es_evi *es_evi;
        struct listnode *node = NULL;
        struct listnode *nnode = NULL;
        struct zebra_if *zif;
+       struct zebra_evpn_es *es = *esp;
 
        if (!CHECK_FLAG(es->flags, ZEBRA_EVPNES_LOCAL))
                return;
@@ -1401,12 +1405,14 @@ static void zebra_evpn_local_es_del(struct zebra_evpn_es *es)
        if (es->flags & ZEBRA_EVPNES_READY_FOR_BGP)
                zebra_evpn_es_send_del_to_client(es);
 
-       zebra_evpn_es_local_info_clear(es);
+       zebra_evpn_es_local_info_clear(esp);
 }
 
 /* eval remote info associated with the ES */
-static void zebra_evpn_es_remote_info_re_eval(struct zebra_evpn_es *es)
+static void zebra_evpn_es_remote_info_re_eval(struct zebra_evpn_es **esp)
 {
+       struct zebra_evpn_es *es = *esp;
+
        /* if there are remote VTEPs the ES-EVI is classified as "remote" */
        if (listcount(es->es_vtep_list)) {
                if (!(es->flags & ZEBRA_EVPNES_REMOTE)) {
@@ -1421,7 +1427,7 @@ static void zebra_evpn_es_remote_info_re_eval(struct zebra_evpn_es *es)
                        if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
                                zlog_debug("remote es %s del; nhg 0x%x",
                                                es->esi_str, es->nhg_id);
-                       zebra_evpn_es_free(es);
+                       zebra_evpn_es_free(esp);
                }
        }
 }
@@ -1442,7 +1448,7 @@ static int zebra_evpn_local_es_update(struct zebra_if *zif, uint32_t lid,
        if (!lid || is_zero_mac(sysmac)) {
                /* if in ES is attached to zif delete it */
                if (old_es)
-                       zebra_evpn_local_es_del(old_es);
+                       zebra_evpn_local_es_del(&zif->es_info.es);
                return 0;
        }
 
@@ -1467,7 +1473,7 @@ static int zebra_evpn_local_es_update(struct zebra_if *zif, uint32_t lid,
 
        /* release the old_es against the zif */
        if (old_es)
-               zebra_evpn_local_es_del(old_es);
+               zebra_evpn_local_es_del(&zif->es_info.es);
 
        es = zebra_evpn_es_find(&esi);
        if (es) {
@@ -1497,23 +1503,23 @@ static int zebra_evpn_remote_es_del(esi_t *esi, struct in_addr vtep_ip)
        es = zebra_evpn_es_find(esi);
        if (!es) {
                zlog_warn("remote es %s vtep %pI4 del failed, es missing",
-                               esi_to_str(esi, buf, sizeof(buf)),
-                               &vtep_ip);
+                         esi_to_str(esi, buf, sizeof(buf)), &vtep_ip);
                return -1;
        }
 
        zebra_evpn_es_vtep_del(es, vtep_ip);
-       zebra_evpn_es_remote_info_re_eval(es);
+       zebra_evpn_es_remote_info_re_eval(&es);
 
        return 0;
 }
 
 /* force delete a remote ES on the way down */
-static void zebra_evpn_remote_es_flush(struct zebra_evpn_es *es)
+static void zebra_evpn_remote_es_flush(struct zebra_evpn_es **esp)
 {
        struct zebra_evpn_es_vtep *es_vtep;
        struct listnode *node;
        struct listnode *nnode;
+       struct zebra_evpn_es *es = *esp;
 
        for (ALL_LIST_ELEMENTS(es->es_vtep_list, node, nnode, es_vtep)) {
                if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
@@ -1521,8 +1527,8 @@ static void zebra_evpn_remote_es_flush(struct zebra_evpn_es *es)
                                        es->esi_str,
                                        inet_ntoa(es_vtep->vtep_ip));
                zebra_evpn_es_vtep_free(es_vtep);
-               zebra_evpn_es_remote_info_re_eval(es);
        }
+       zebra_evpn_es_remote_info_re_eval(esp);
 }
 
 static int zebra_evpn_remote_es_add(esi_t *esi, struct in_addr vtep_ip)
@@ -1539,15 +1545,15 @@ static int zebra_evpn_remote_es_add(esi_t *esi, struct in_addr vtep_ip)
        if (!es) {
                es = zebra_evpn_es_new(esi);
                if (!es) {
-                       zlog_warn("remote es %s vtep %pI4 add failed, es missing",
-                                       esi_to_str(esi, buf, sizeof(buf)),
-                                       &vtep_ip);
+                       zlog_warn(
+                               "remote es %s vtep %pI4 add failed, es missing",
+                               esi_to_str(esi, buf, sizeof(buf)), &vtep_ip);
                        return -1;
                }
        }
 
        zebra_evpn_es_vtep_add(es, vtep_ip);
-       zebra_evpn_es_remote_info_re_eval(es);
+       zebra_evpn_es_remote_info_re_eval(&es);
 
        return 0;
 }
@@ -1587,7 +1593,7 @@ void zebra_evpn_es_mac_deref_entry(zebra_mac_t *mac)
 
        list_delete_node(es->mac_list, &mac->es_listnode);
        if (!listcount(es->mac_list))
-               zebra_evpn_es_free(es);
+               zebra_evpn_es_free(&es);
 }
 
 /* Associate a MAC entry with a local or remote ES. Returns false if there
@@ -1694,8 +1700,9 @@ void zebra_evpn_es_cleanup(void)
 
        RB_FOREACH_SAFE(es, zebra_es_rb_head,
                        &zmh_info->es_rb_tree, es_next) {
-               zebra_evpn_local_es_del(es);
-               zebra_evpn_remote_es_flush(es);
+               zebra_evpn_local_es_del(&es);
+               if (es)
+                       zebra_evpn_remote_es_flush(&es);
        }
 }