Skip to content

Commit

Permalink
zebra: fix use of freed es during zebra shutdown
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@AnuradhaKaruppiah @idryzhov
AnuradhaKaruppiah authored and idryzhov committed Oct 6, 2020
1 parent 81cd4fb commit 51c56f8
Showing 1 changed file with 32 additions and 25 deletions.
57 changes: 32 additions & 25 deletions zebra/zebra_evpn_mh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}

/*****************************************************************************
Expand Down Expand Up @@ -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);
Expand All @@ -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 */
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)) {
Expand All @@ -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);
}
}
}
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -1497,32 +1503,32 @@ 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)
zlog_debug("es %s vtep %s flush",
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)
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 51c56f8

Please sign in to comment.