Skip to content

Commit

Permalink
bgpd: Prevent usage after free in bgp_mac.c
Browse files Browse the repository at this point in the history
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
    FRRouting#10 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
    FRRouting#11 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
    FRRouting#12 0x55e33d09d463 in main bgpd/bgp_main.c:477
    FRRouting#13 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
    FRRouting#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
    FRRouting#10 0x55e33d09d463 in main bgpd/bgp_main.c:477
    FRRouting#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
    FRRouting#10 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
    FRRouting#11 0x55e33d09d463 in main bgpd/bgp_main.c:477
    FRRouting#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 <[email protected]>
  • Loading branch information
donaldsharp committed Nov 1, 2019
1 parent f149ebd commit c86e8d6
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion bgpd/bgp_mac.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit c86e8d6

Please sign in to comment.