From c86e8d64e81bf4dd3d7301685402d3d55ae9697d Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Thu, 31 Oct 2019 20:23:23 -0400
Subject: [PATCH] bgpd: Prevent usage after free in bgp_mac.c

Running with --enable-address-sanitizer I am seeing this:

=================================================================
==19520==ERROR: AddressSanitizer: heap-use-after-free on address 0x6020003ef850 at pc 0x7fe9b8f7b57b bp 0x7fffbac6f9c0 sp 0x7fffbac6f170
READ of size 6 at 0x6020003ef850 thread T0
    #0 0x7fe9b8f7b57a  (/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
    #1 0x55e33d1071e5 in bgp_process_mac_rescan_table bgpd/bgp_mac.c:159
    #2 0x55e33d107c09 in bgp_mac_rescan_evpn_table bgpd/bgp_mac.c:252
    #3 0x55e33d107e39 in bgp_mac_rescan_all_evpn_tables bgpd/bgp_mac.c:266
    #4 0x55e33d108270 in bgp_mac_remove_ifp_internal bgpd/bgp_mac.c:291
    #5 0x55e33d108893 in bgp_mac_del_mac_entry bgpd/bgp_mac.c:351
    #6 0x55e33d21412d in bgp_ifp_down bgpd/bgp_zebra.c:257
    #7 0x7fe9b8cbf3be in if_down_via_zapi lib/if.c:198
    #8 0x7fe9b8db303a in zclient_interface_down lib/zclient.c:1549
    #9 0x7fe9b8db8a06 in zclient_read lib/zclient.c:2693
    #10 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
    #11 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
    #12 0x55e33d09d463 in main bgpd/bgp_main.c:477
    #13 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
    #14 0x55e33d09c189 in _start (/usr/lib/frr/bgpd+0x168189)
0x6020003ef850 is located 0 bytes inside of 16-byte region [0x6020003ef850,0x6020003ef860)
freed by thread T0 here:
    #0 0x7fe9b8fabfb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x7fe9b8ce4ea9 in qfree lib/memory.c:129
    #2 0x55e33d10825c in bgp_mac_remove_ifp_internal bgpd/bgp_mac.c:289
    #3 0x55e33d108893 in bgp_mac_del_mac_entry bgpd/bgp_mac.c:351
    #4 0x55e33d21412d in bgp_ifp_down bgpd/bgp_zebra.c:257
    #5 0x7fe9b8cbf3be in if_down_via_zapi lib/if.c:198
    #6 0x7fe9b8db303a in zclient_interface_down lib/zclient.c:1549
    #7 0x7fe9b8db8a06 in zclient_read lib/zclient.c:2693
    #8 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
    #9 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
    #10 0x55e33d09d463 in main bgpd/bgp_main.c:477
    #11 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
    #0 0x7fe9b8fac518 in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0xe9518)
    #1 0x7fe9b8ce4d93 in qcalloc lib/memory.c:110
    #2 0x55e33d106b29 in bgp_mac_hash_alloc bgpd/bgp_mac.c:96
    #3 0x7fe9b8cb8350 in hash_get lib/hash.c:149
    #4 0x55e33d10845b in bgp_mac_add_mac_entry bgpd/bgp_mac.c:303
    #5 0x55e33d226757 in bgp_ifp_create bgpd/bgp_zebra.c:2644
    #6 0x7fe9b8cbf1e6 in if_new_via_zapi lib/if.c:176
    #7 0x7fe9b8db2d3b in zclient_interface_add lib/zclient.c:1481
    #8 0x7fe9b8db87f8 in zclient_read lib/zclient.c:2659
    #9 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
    #10 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
    #11 0x55e33d09d463 in main bgpd/bgp_main.c:477
    #12 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308

Effectively we are passing to bgp_mac_remove_ifp_internal the macaddr
that is associated with the bsm data structure.  There exists a path
where the bsm is freed and then we immediately pass the macaddr into
bgp_mac_rescan_all_evpn_tables.  So just make a copy of the macaddr
data structure before we free the bsm

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
 bgpd/bgp_mac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bgpd/bgp_mac.c b/bgpd/bgp_mac.c
index 61c7b4080c86..537bb45455e8 100644
--- a/bgpd/bgp_mac.c
+++ b/bgpd/bgp_mac.c
@@ -284,11 +284,13 @@ static void bgp_mac_remove_ifp_internal(struct bgp_self_mac *bsm, char *ifname,
 	}
 
 	if (bsm->ifp_list->count == 0) {
+		struct ethaddr mac = *macaddr;
+
 		hash_release(bm->self_mac_hash, bsm);
 		list_delete(&bsm->ifp_list);
 		XFREE(MTYPE_BSM, bsm);
 
-		bgp_mac_rescan_all_evpn_tables(macaddr);
+		bgp_mac_rescan_all_evpn_tables(&mac);
 	}
 }