Skip to content

pbrd: add meat of rule range cli#5

Merged
donaldsharp merged 1 commit intodonaldsharp:PBRDfrom
dslicenc:PBRD
Feb 12, 2018
Merged

pbrd: add meat of rule range cli#5
donaldsharp merged 1 commit intodonaldsharp:PBRDfrom
dslicenc:PBRD

Conversation

@dslicenc
Copy link
Copy Markdown

Signed-off-by: Don Slice dslice@cumulusnetworks.com

Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
@donaldsharp donaldsharp merged commit 382e388 into donaldsharp:PBRD Feb 12, 2018
@dslicenc dslicenc deleted the PBRD branch February 12, 2018 23:29
donaldsharp pushed a commit that referenced this pull request Jun 19, 2018
donaldsharp pushed a commit that referenced this pull request Feb 24, 2019
If path->net is NULL in the bgp_path_info_free() function, then
bgpd would crash in bgp_addpath_free_info_data() with the following
backtrace:

 (gdb) bt
 #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
 #1  0x00007ff7b267a42a in __GI_abort () at abort.c:89
 #2  0x00007ff7b39c1ca0 in core_handler (signo=11, siginfo=0x7ffff66414f0, context=<optimized out>) at lib/sigevent.c:249
 #3  <signal handler called>
 #4  idalloc_free_to_pool (pool_ptr=pool_ptr@entry=0x0, id=3) at lib/id_alloc.c:368
 #5  0x0000560096246688 in bgp_addpath_free_info_data (d=d@entry=0x560098665468, nd=0x0) at bgpd/bgp_addpath.c:100
 #6  0x00005600961bb522 in bgp_path_info_free (path=0x560098665400) at bgpd/bgp_route.c:252
 #7  bgp_path_info_unlock (path=0x560098665400) at bgpd/bgp_route.c:276
 #8  0x00005600961bb719 in bgp_path_info_reap (rn=rn@entry=0x5600986b2110, pi=pi@entry=0x560098665400) at bgpd/bgp_route.c:320
 #9  0x00005600961bf4db in bgp_process_main_one (safi=SAFI_MPLS_VPN, afi=AFI_IP, rn=0x5600986b2110, bgp=0x560098587320) at bgpd/bgp_route.c:2476
 #10 bgp_process_wq (wq=<optimized out>, data=0x56009869b8f0) at bgpd/bgp_route.c:2503
 #11 0x00007ff7b39d5fcc in work_queue_run (thread=0x7ffff6641e10) at lib/workqueue.c:294
 #12 0x00007ff7b39ce3b1 in thread_call (thread=thread@entry=0x7ffff6641e10) at lib/thread.c:1606
 #13 0x00007ff7b39a3538 in frr_run (master=0x5600980795b0) at lib/libfrr.c:1011
 #14 0x000056009618a5a3 in main (argc=3, argv=0x7ffff6642078) at bgpd/bgp_main.c:481

Add a null-check protection to fix this problem.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
donaldsharp pushed a commit that referenced this pull request Feb 24, 2019
If path->net is NULL in the bgp_path_info_free() function, then
bgpd would crash in bgp_addpath_free_info_data() with the following
backtrace:

 (gdb) bt
 #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
 #1  0x00007ff7b267a42a in __GI_abort () at abort.c:89
 #2  0x00007ff7b39c1ca0 in core_handler (signo=11, siginfo=0x7ffff66414f0, context=<optimized out>) at lib/sigevent.c:249
 #3  <signal handler called>
 #4  idalloc_free_to_pool (pool_ptr=pool_ptr@entry=0x0, id=3) at lib/id_alloc.c:368
 #5  0x0000560096246688 in bgp_addpath_free_info_data (d=d@entry=0x560098665468, nd=0x0) at bgpd/bgp_addpath.c:100
 #6  0x00005600961bb522 in bgp_path_info_free (path=0x560098665400) at bgpd/bgp_route.c:252
 #7  bgp_path_info_unlock (path=0x560098665400) at bgpd/bgp_route.c:276
 #8  0x00005600961bb719 in bgp_path_info_reap (rn=rn@entry=0x5600986b2110, pi=pi@entry=0x560098665400) at bgpd/bgp_route.c:320
 #9  0x00005600961bf4db in bgp_process_main_one (safi=SAFI_MPLS_VPN, afi=AFI_IP, rn=0x5600986b2110, bgp=0x560098587320) at bgpd/bgp_route.c:2476
 #10 bgp_process_wq (wq=<optimized out>, data=0x56009869b8f0) at bgpd/bgp_route.c:2503
 #11 0x00007ff7b39d5fcc in work_queue_run (thread=0x7ffff6641e10) at lib/workqueue.c:294
 #12 0x00007ff7b39ce3b1 in thread_call (thread=thread@entry=0x7ffff6641e10) at lib/thread.c:1606
 #13 0x00007ff7b39a3538 in frr_run (master=0x5600980795b0) at lib/libfrr.c:1011
 #14 0x000056009618a5a3 in main (argc=3, argv=0x7ffff6642078) at bgpd/bgp_main.c:481

Add a null-check protection to fix this problem.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
donaldsharp added a commit that referenced this pull request Oct 10, 2019
Our Address Sanitizer CI is finding this issue:
error	09-Oct-2019 19:28:33	r4: bgpd triggered an exception by AddressSanitizer
error	09-Oct-2019 19:28:33	ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdd425b060 at pc 0x00000068575f bp 0x7ffdd4258550 sp 0x7ffdd4258540
error	09-Oct-2019 19:28:33	READ of size 1 at 0x7ffdd425b060 thread T0
error	09-Oct-2019 19:28:33	    #0 0x68575e in prefix_cmp lib/prefix.c:776
error	09-Oct-2019 19:28:33	    #1 0x5889f5 in rfapiItBiIndexSearch bgpd/rfapi/rfapi_import.c:2230
error	09-Oct-2019 19:28:33	    #2 0x5889f5 in rfapiBgpInfoFilteredImportVPN bgpd/rfapi/rfapi_import.c:3520
error	09-Oct-2019 19:28:33	    #3 0x58b909 in rfapiProcessWithdraw bgpd/rfapi/rfapi_import.c:4071
error	09-Oct-2019 19:28:33	    #4 0x4c459b in bgp_withdraw bgpd/bgp_route.c:3736
error	09-Oct-2019 19:28:33	    #5 0x484122 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:237
error	09-Oct-2019 19:28:33	    #6 0x497f52 in bgp_nlri_parse bgpd/bgp_packet.c:315
error	09-Oct-2019 19:28:33	    #7 0x49d06d in bgp_update_receive bgpd/bgp_packet.c:1598
error	09-Oct-2019 19:28:33	    #8 0x49d06d in bgp_process_packet bgpd/bgp_packet.c:2274
error	09-Oct-2019 19:28:33	    #9 0x6b9f54 in thread_call lib/thread.c:1531
error	09-Oct-2019 19:28:33	    #10 0x657037 in frr_run lib/libfrr.c:1052
error	09-Oct-2019 19:28:33	    #11 0x42d268 in main bgpd/bgp_main.c:486
error	09-Oct-2019 19:28:33	    #12 0x7f806032482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
error	09-Oct-2019 19:28:33	    #13 0x42bcc8 in _start (/usr/lib/frr/bgpd+0x42bcc8)
error	09-Oct-2019 19:28:33
error	09-Oct-2019 19:28:33	Address 0x7ffdd425b060 is located in stack of thread T0 at offset 240 in frame
error	09-Oct-2019 19:28:33	    #0 0x483945 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:103
error	09-Oct-2019 19:28:33
error	09-Oct-2019 19:28:33	  This frame has 5 object(s):
error	09-Oct-2019 19:28:33	    [32, 36) 'label'
error	09-Oct-2019 19:28:33	    [96, 108) 'rd_as'
error	09-Oct-2019 19:28:33	    [160, 172) 'rd_ip'
error	09-Oct-2019 19:28:33	    [224, 240) 'prd' <== Memory access at offset 240 overflows this variable
error	09-Oct-2019 19:28:33	    [288, 336) 'p'
error	09-Oct-2019 19:28:33	HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
error	09-Oct-2019 19:28:33	      (longjmp and C++ exceptions *are* supported)
error	09-Oct-2019 19:28:33	SUMMARY: AddressSanitizer: stack-buffer-overflow lib/prefix.c:776 prefix_cmp
error	09-Oct-2019 19:28:33	Shadow bytes around the buggy address:
error	09-Oct-2019 19:28:33	  0x10003a8435b0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a8435c0: 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 f3
error	09-Oct-2019 19:28:33	  0x10003a8435d0: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a8435e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
error	09-Oct-2019 19:28:33	  0x10003a8435f0: f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 04 f4 f4 f2 f2
error	09-Oct-2019 19:28:33	=>0x10003a843600: f2 f2 00 04 f4 f4 f2 f2 f2 f2 00 00[f4]f4 f2 f2
error	09-Oct-2019 19:28:33	  0x10003a843610: f2 f2 00 00 00 00 00 00 f4 f4 f3 f3 f3 f3 00 00
error	09-Oct-2019 19:28:33	  0x10003a843620: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a843630: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 02 f4
error	09-Oct-2019 19:28:33	  0x10003a843640: f4 f4 f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00
error	09-Oct-2019 19:28:33	  0x10003a843650: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
error	09-Oct-2019 19:28:33	Shadow byte legend (one shadow byte represents 8 application bytes):
error	09-Oct-2019 19:28:33	  Addressable:           00
error	09-Oct-2019 19:28:33	  Partially addressable: 01 02 03 04 05 06 07
error	09-Oct-2019 19:28:33	  Heap left redzone:       fa
error	09-Oct-2019 19:28:33	  Heap right redzone:      fb
error	09-Oct-2019 19:28:33	  Freed heap region:       fd
error	09-Oct-2019 19:28:33	  Stack left redzone:      f1
error	09-Oct-2019 19:28:33	  Stack mid redzone:       f2
error	09-Oct-2019 19:28:33	  Stack right redzone:     f3
error	09-Oct-2019 19:28:33	  Stack partial redzone:   f4
error	09-Oct-2019 19:28:33	  Stack after return:      f5
error	09-Oct-2019 19:28:33	  Stack use after scope:   f8
error	09-Oct-2019 19:28:33	  Global redzone:          f9
error	09-Oct-2019 19:28:33	  Global init order:       f6
error	09-Oct-2019 19:28:33	  Poisoned by user:        f7
error	09-Oct-2019 19:28:33	  Container overflow:      fc
error	09-Oct-2019 19:28:33	  Array cookie:            ac
error	09-Oct-2019 19:28:33	  Intra object redzone:    bb
error	09-Oct-2019 19:28:33	  ASan internal:           fe
error	09-Oct-2019 19:28:36	r3: Daemon bgpd not running

This is the result of this code pattern in rfapi/rfapi_import.c:

prefix_cmp((struct prefix *)&bpi_result->extra->vnc.import.rd,
	   (struct prefix *)prd))

Effectively prd or vnc.import.rd are `struct prefix_rd` which
are being typecast to a `struct prefix`.  Not a big deal except commit
1315d74 modified the prefix_cmp
function to allow for a sorted prefix_cmp.  In prefix_cmp
we were looking at the offset and shift.  In the case
of vnc we were passing a prefix length of 64 which is the exact length of
the remaining data structure for struct prefix_rd.  So we calculated
a offset of 8 and a shift of 0.  The data structures for the prefix
portion happened to be equal to 64 bits of data. So we checked that
with the memcmp got a 0 and promptly read off the end of the data
structure for the numcmp.  The fix is if shift is 0 that means thei
the memcmp has checked everything and there is nothing to do.

Please note: We will still crash if we set the prefixlen > then
~312 bits currently( ie if the prefixlen specifies a bit length
longer than the prefix length ).  I do not think there is
anything to do here( nor am I sure how to correct this either )
as that we are going to have some severe problems when we muck
up the prefixlen.

Fixes: FRRouting#5025
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Oct 15, 2019
Our Address Sanitizer CI is finding this issue:
error	09-Oct-2019 19:28:33	r4: bgpd triggered an exception by AddressSanitizer
error	09-Oct-2019 19:28:33	ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdd425b060 at pc 0x00000068575f bp 0x7ffdd4258550 sp 0x7ffdd4258540
error	09-Oct-2019 19:28:33	READ of size 1 at 0x7ffdd425b060 thread T0
error	09-Oct-2019 19:28:33	    #0 0x68575e in prefix_cmp lib/prefix.c:776
error	09-Oct-2019 19:28:33	    #1 0x5889f5 in rfapiItBiIndexSearch bgpd/rfapi/rfapi_import.c:2230
error	09-Oct-2019 19:28:33	    #2 0x5889f5 in rfapiBgpInfoFilteredImportVPN bgpd/rfapi/rfapi_import.c:3520
error	09-Oct-2019 19:28:33	    #3 0x58b909 in rfapiProcessWithdraw bgpd/rfapi/rfapi_import.c:4071
error	09-Oct-2019 19:28:33	    #4 0x4c459b in bgp_withdraw bgpd/bgp_route.c:3736
error	09-Oct-2019 19:28:33	    #5 0x484122 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:237
error	09-Oct-2019 19:28:33	    #6 0x497f52 in bgp_nlri_parse bgpd/bgp_packet.c:315
error	09-Oct-2019 19:28:33	    #7 0x49d06d in bgp_update_receive bgpd/bgp_packet.c:1598
error	09-Oct-2019 19:28:33	    #8 0x49d06d in bgp_process_packet bgpd/bgp_packet.c:2274
error	09-Oct-2019 19:28:33	    #9 0x6b9f54 in thread_call lib/thread.c:1531
error	09-Oct-2019 19:28:33	    #10 0x657037 in frr_run lib/libfrr.c:1052
error	09-Oct-2019 19:28:33	    #11 0x42d268 in main bgpd/bgp_main.c:486
error	09-Oct-2019 19:28:33	    #12 0x7f806032482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
error	09-Oct-2019 19:28:33	    #13 0x42bcc8 in _start (/usr/lib/frr/bgpd+0x42bcc8)
error	09-Oct-2019 19:28:33
error	09-Oct-2019 19:28:33	Address 0x7ffdd425b060 is located in stack of thread T0 at offset 240 in frame
error	09-Oct-2019 19:28:33	    #0 0x483945 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:103
error	09-Oct-2019 19:28:33
error	09-Oct-2019 19:28:33	  This frame has 5 object(s):
error	09-Oct-2019 19:28:33	    [32, 36) 'label'
error	09-Oct-2019 19:28:33	    [96, 108) 'rd_as'
error	09-Oct-2019 19:28:33	    [160, 172) 'rd_ip'
error	09-Oct-2019 19:28:33	    [224, 240) 'prd' <== Memory access at offset 240 overflows this variable
error	09-Oct-2019 19:28:33	    [288, 336) 'p'
error	09-Oct-2019 19:28:33	HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
error	09-Oct-2019 19:28:33	      (longjmp and C++ exceptions *are* supported)
error	09-Oct-2019 19:28:33	SUMMARY: AddressSanitizer: stack-buffer-overflow lib/prefix.c:776 prefix_cmp
error	09-Oct-2019 19:28:33	Shadow bytes around the buggy address:
error	09-Oct-2019 19:28:33	  0x10003a8435b0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a8435c0: 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 f3
error	09-Oct-2019 19:28:33	  0x10003a8435d0: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a8435e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
error	09-Oct-2019 19:28:33	  0x10003a8435f0: f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 04 f4 f4 f2 f2
error	09-Oct-2019 19:28:33	=>0x10003a843600: f2 f2 00 04 f4 f4 f2 f2 f2 f2 00 00[f4]f4 f2 f2
error	09-Oct-2019 19:28:33	  0x10003a843610: f2 f2 00 00 00 00 00 00 f4 f4 f3 f3 f3 f3 00 00
error	09-Oct-2019 19:28:33	  0x10003a843620: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
error	09-Oct-2019 19:28:33	  0x10003a843630: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 02 f4
error	09-Oct-2019 19:28:33	  0x10003a843640: f4 f4 f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00
error	09-Oct-2019 19:28:33	  0x10003a843650: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
error	09-Oct-2019 19:28:33	Shadow byte legend (one shadow byte represents 8 application bytes):
error	09-Oct-2019 19:28:33	  Addressable:           00
error	09-Oct-2019 19:28:33	  Partially addressable: 01 02 03 04 05 06 07
error	09-Oct-2019 19:28:33	  Heap left redzone:       fa
error	09-Oct-2019 19:28:33	  Heap right redzone:      fb
error	09-Oct-2019 19:28:33	  Freed heap region:       fd
error	09-Oct-2019 19:28:33	  Stack left redzone:      f1
error	09-Oct-2019 19:28:33	  Stack mid redzone:       f2
error	09-Oct-2019 19:28:33	  Stack right redzone:     f3
error	09-Oct-2019 19:28:33	  Stack partial redzone:   f4
error	09-Oct-2019 19:28:33	  Stack after return:      f5
error	09-Oct-2019 19:28:33	  Stack use after scope:   f8
error	09-Oct-2019 19:28:33	  Global redzone:          f9
error	09-Oct-2019 19:28:33	  Global init order:       f6
error	09-Oct-2019 19:28:33	  Poisoned by user:        f7
error	09-Oct-2019 19:28:33	  Container overflow:      fc
error	09-Oct-2019 19:28:33	  Array cookie:            ac
error	09-Oct-2019 19:28:33	  Intra object redzone:    bb
error	09-Oct-2019 19:28:33	  ASan internal:           fe
error	09-Oct-2019 19:28:36	r3: Daemon bgpd not running

This is the result of this code pattern in rfapi/rfapi_import.c:

prefix_cmp((struct prefix *)&bpi_result->extra->vnc.import.rd,
	   (struct prefix *)prd))

Effectively prd or vnc.import.rd are `struct prefix_rd` which
are being typecast to a `struct prefix`.  Not a big deal except commit
1315d74 modified the prefix_cmp
function to allow for a sorted prefix_cmp.  In prefix_cmp
we were looking at the offset and shift.  In the case
of vnc we were passing a prefix length of 64 which is the exact length of
the remaining data structure for struct prefix_rd.  So we calculated
a offset of 8 and a shift of 0.  The data structures for the prefix
portion happened to be equal to 64 bits of data. So we checked that
with the memcmp got a 0 and promptly read off the end of the data
structure for the numcmp.  The fix is if shift is 0 that means thei
the memcmp has checked everything and there is nothing to do.

Please note: We will still crash if we set the prefixlen > then
~312 bits currently( ie if the prefixlen specifies a bit length
longer than the prefix length ).  I do not think there is
anything to do here( nor am I sure how to correct this either )
as that we are going to have some severe problems when we muck
up the prefixlen.

Fixes: FRRouting#5025
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Nov 1, 2019
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>
donaldsharp pushed a commit that referenced this pull request Nov 15, 2019
This code is called from the zebra main pthread during shutdown
but the thread event is scheduled via the zebra dplane pthread.

Hence, we should be using the `thread_cancel_async()` API to
cancel the thread event on a different pthread.

This is only ever hit in the rare case that we still have work left
to do on the update queue during shutdown.

Found via zebra crash:

```
(gdb) bt
\#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
\#1  0x00007f4e4d3f7535 in __GI_abort () at abort.c:79
\#2  0x00007f4e4d3f740f in __assert_fail_base (fmt=0x7f4e4d559ee0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7f4e4d9071d0 "master->owner == pthread_self()",
    file=0x7f4e4d906cf8 "lib/thread.c", line=1185, function=<optimized out>) at assert.c:92
\#3  0x00007f4e4d405102 in __GI___assert_fail (assertion=assertion@entry=0x7f4e4d9071d0 "master->owner == pthread_self()", file=file@entry=0x7f4e4d906cf8 "lib/thread.c",
    line=line@entry=1185, function=function@entry=0x7f4e4d906b68 <__PRETTY_FUNCTION__.15817> "thread_cancel") at assert.c:101
\#4  0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185
\#5  0x000055b40c291845 in zebra_dplane_shutdown () at zebra/zebra_dplane.c:3274
\#6  0x000055b40c27ee13 in zebra_finalize (dummy=<optimized out>) at zebra/main.c:202
\#7  0x00007f4e4d8d1416 in thread_call (thread=thread@entry=0x7ffcbbc08870) at lib/thread.c:1599
\#8  0x00007f4e4d8a1ef8 in frr_run (master=0x55b40ce35510) at lib/libfrr.c:1024
\#9  0x000055b40c270916 in main (argc=8, argv=0x7ffcbbc08c78) at zebra/main.c:483
(gdb) down
\#4  0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185
1185		assert(master->owner == pthread_self());
(gdb)
```

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Dec 18, 2019
Address Sanitizer is reporting this issue:

==26177==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000238d8 at pc 0x7f88f7c4fa93 bp 0x7fff9a641830 sp 0x7fff9a641820
READ of size 8 at 0x6120000238d8 thread T0
    #0 0x7f88f7c4fa92 in if_delete lib/if.c:290
    #1 0x42192e in ospf_vl_if_delete ospfd/ospf_interface.c:912
    #2 0x42192e in ospf_vl_delete ospfd/ospf_interface.c:990
    #3 0x4a6208 in no_ospf_area_vlink ospfd/ospf_vty.c:1227
    #4 0x7f88f7c1553d in cmd_execute_command_real lib/command.c:1073
    #5 0x7f88f7c19b1e in cmd_execute_command lib/command.c:1132
    #6 0x7f88f7c19e8e in cmd_execute lib/command.c:1288
    #7 0x7f88f7cd7523 in vty_command lib/vty.c:516
    #8 0x7f88f7cd79ff in vty_execute lib/vty.c:1285
    #9 0x7f88f7cde4f9 in vtysh_read lib/vty.c:2119
    #10 0x7f88f7ccb845 in thread_call lib/thread.c:1549
    #11 0x7f88f7c5d6a7 in frr_run lib/libfrr.c:1093
    #12 0x412976 in main ospfd/ospf_main.c:221
    #13 0x7f88f73b082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #14 0x413c78 in _start (/usr/local/master/sbin/ospfd+0x413c78)

Effectively we are in a shutdown phase and as part of shutdown we delete the
ospf interface pointer ( ifp->info ).  The interface deletion code
was modified in the past year to pass in the address of operator
to allow us to NULL out the holding pointer.  The catch here
is that we free the oi and then delete the interface passing
in the address of the oi->ifp pointer, causing a use after free.

Fixes: FRRouting#5555
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Dec 27, 2019
We were not connecting the default zebra_ns to the default
ns->info at namespace initialization in zebra. Thus, when
we tried to use the `ns_walk_func()` it would ignore the
default zebra_ns since there is no pointer to it from the
ns struct.

Fix this by connecting them in `zebra_ns_init()` and,
if the default ns is not found, exit with failure
since this is not recoverable.

This was found during a crash where we fail to cancel the kernel_read
thread at termination (via the `ns_walk_func()`) and then we
get a netlink notification trying to use the zns struct that has
already been freed.

```
(gdb) bt
\#0  0x00007fc1134dc7bb in raise () from /lib/x86_64-linux-gnu/libc.so.6
\#1  0x00007fc1134c7535 in abort () from /lib/x86_64-linux-gnu/libc.so.6
\#2  0x00007fc113996f8f in core_handler (signo=11, siginfo=0x7ffe5429d070, context=<optimized out>) at lib/sigevent.c:254
\#3  <signal handler called>
\#4  0x0000561880e15449 in if_lookup_by_index_per_ns (ns=0x0, ifindex=174) at zebra/interface.c:269
\#5  0x0000561880e1642c in if_up (ifp=ifp@entry=0x561883076c50) at zebra/interface.c:1043
\#6  0x0000561880e10723 in netlink_link_change (h=0x7ffe5429d8f0, ns_id=<optimized out>, startup=<optimized out>) at zebra/if_netlink.c:1384
\#7  0x0000561880e17e68 in netlink_parse_info (filter=filter@entry=0x561880e17680 <netlink_information_fetch>, nl=nl@entry=0x561882497238, zns=zns@entry=0x7ffe542a5940,
    count=count@entry=5, startup=startup@entry=0) at zebra/kernel_netlink.c:932
\#8  0x0000561880e186a5 in kernel_read (thread=<optimized out>) at zebra/kernel_netlink.c:406
\#9  0x00007fc1139a4416 in thread_call (thread=thread@entry=0x7ffe542a5b70) at lib/thread.c:1599
\#10 0x00007fc113974ef8 in frr_run (master=0x5618823c9510) at lib/libfrr.c:1024
\#11 0x0000561880e0b916 in main (argc=8, argv=0x7ffe542a5f78) at zebra/main.c:483
```

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 3, 2020
=================================================================
==13611==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe9e5c8694 at pc 0x0000004d18ac bp 0x7ffe9e5c8330 sp 0x7ffe9e5c7ae0
WRITE of size 17 at 0x7ffe9e5c8694 thread T0
    #0 0x4d18ab in __asan_memcpy (/usr/lib/frr/zebra+0x4d18ab)
    #1 0x7f16f04bd97f in stream_get2 /home/qlyoung/frr/lib/stream.c:277:2
    #2 0x6410ec in zebra_vxlan_remote_macip_del /home/qlyoung/frr/zebra/zebra_vxlan.c:7718:4
    #3 0x68fa98 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
    #4 0x556add in main /home/qlyoung/frr/zebra/main.c:309:2
    #5 0x7f16eea3bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #6 0x431249 in _start (/usr/lib/frr/zebra+0x431249)
donaldsharp added a commit that referenced this pull request Jan 3, 2020
=================================================================
==13611==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe9e5c8694 at pc 0x0000004d18ac bp 0x7ffe9e5c8330 sp 0x7ffe9e5c7ae0
WRITE of size 17 at 0x7ffe9e5c8694 thread T0
    #0 0x4d18ab in __asan_memcpy (/usr/lib/frr/zebra+0x4d18ab)
    #1 0x7f16f04bd97f in stream_get2 /home/qlyoung/frr/lib/stream.c:277:2
    #2 0x6410ec in zebra_vxlan_remote_macip_del /home/qlyoung/frr/zebra/zebra_vxlan.c:7718:4
    #3 0x68fa98 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
    #4 0x556add in main /home/qlyoung/frr/zebra/main.c:309:2
    #5 0x7f16eea3bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #6 0x431249 in _start (/usr/lib/frr/zebra+0x431249)

This decode is the result of a buffer overflow because we are
not checking ipa_len.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 3, 2020
=================================================================
==3058==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f5bf3ef7477 bp 0x7ffdfaa20d40 sp 0x7ffdfaa204c8 T0)
==3058==The signal is caused by a READ memory access.
==3058==Hint: address points to the zero page.
    #0 0x7f5bf3ef7476 in memcpy /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:134
    #1 0x4d158a in __asan_memcpy (/usr/lib/frr/zebra+0x4d158a)
    #2 0x7f5bf58da8ad in stream_put /home/qlyoung/frr/lib/stream.c:605:3
    #3 0x67d428 in zsend_ipset_entry_notify_owner /home/qlyoung/frr/zebra/zapi_msg.c:851:2
    #4 0x5c70b3 in zebra_pbr_add_ipset_entry /home/qlyoung/frr/zebra/zebra_pbr.c
    #5 0x68e1bb in zread_ipset_entry /home/qlyoung/frr/zebra/zapi_msg.c:2465:4
    #6 0x68f958 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
    #7 0x55666d in main /home/qlyoung/frr/zebra/main.c:309:2
    #8 0x7f5bf3e5db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #9 0x4311d9 in _start (/usr/lib/frr/zebra+0x4311d9)

the ipset->backpointer was NULL as that the hash lookup failed to find
anything.  Prevent this crash from happening.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 7, 2020
=================================================================
==13611==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe9e5c8694 at pc 0x0000004d18ac bp 0x7ffe9e5c8330 sp 0x7ffe9e5c7ae0
WRITE of size 17 at 0x7ffe9e5c8694 thread T0
    #0 0x4d18ab in __asan_memcpy (/usr/lib/frr/zebra+0x4d18ab)
    #1 0x7f16f04bd97f in stream_get2 /home/qlyoung/frr/lib/stream.c:277:2
    #2 0x6410ec in zebra_vxlan_remote_macip_del /home/qlyoung/frr/zebra/zebra_vxlan.c:7718:4
    #3 0x68fa98 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
    #4 0x556add in main /home/qlyoung/frr/zebra/main.c:309:2
    #5 0x7f16eea3bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #6 0x431249 in _start (/usr/lib/frr/zebra+0x431249)

This decode is the result of a buffer overflow because we are
not checking ipa_len.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 7, 2020
=================================================================
==3058==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f5bf3ef7477 bp 0x7ffdfaa20d40 sp 0x7ffdfaa204c8 T0)
==3058==The signal is caused by a READ memory access.
==3058==Hint: address points to the zero page.
    #0 0x7f5bf3ef7476 in memcpy /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:134
    #1 0x4d158a in __asan_memcpy (/usr/lib/frr/zebra+0x4d158a)
    #2 0x7f5bf58da8ad in stream_put /home/qlyoung/frr/lib/stream.c:605:3
    #3 0x67d428 in zsend_ipset_entry_notify_owner /home/qlyoung/frr/zebra/zapi_msg.c:851:2
    #4 0x5c70b3 in zebra_pbr_add_ipset_entry /home/qlyoung/frr/zebra/zebra_pbr.c
    #5 0x68e1bb in zread_ipset_entry /home/qlyoung/frr/zebra/zapi_msg.c:2465:4
    #6 0x68f958 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
    #7 0x55666d in main /home/qlyoung/frr/zebra/main.c:309:2
    #8 0x7f5bf3e5db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #9 0x4311d9 in _start (/usr/lib/frr/zebra+0x4311d9)

the ipset->backpointer was NULL as that the hash lookup failed to find
anything.  Prevent this crash from happening.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Jan 9, 2020
ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16416 byte(s) in 1 object(s) allocated from:
    #0 0x7f08e3ca8602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x7f08e389c4b0 in qmalloc lib/memory.c:105
    #2 0x7f08e38e87b4 in stream_new lib/stream.c:106
    #3 0x481d7f in _zebra_ptm_bfd_client_deregister zebra/zebra_ptm.c:1348
    #4 0x4e7b84 in hook_call_zserv_client_close zebra/zserv.c:544
    #5 0x4e7b84 in zserv_client_free zebra/zserv.c:560
    #6 0x4e7b84 in zserv_close_client zebra/zserv.c:625
    #7 0x4e7fe0 in zserv_handle_client_fail zebra/zserv.c:638
    #8 0x7f08e3901995 in thread_call lib/thread.c:1549
    #9 0x7f08e38937d7 in frr_run lib/libfrr.c:1093
    #10 0x41686e in main zebra/main.c:470
    #11 0x7f08e2fe682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Direct leak of 16416 byte(s) in 1 object(s) allocated from:
    #0 0x7f08e3ca8602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x7f08e389c4b0 in qmalloc lib/memory.c:105
    #2 0x7f08e38e87b4 in stream_new lib/stream.c:106
    #3 0x481efe in _zebra_ptm_reroute zebra/zebra_ptm.c:1411
    #4 0x4f7dc9 in zserv_handle_commands zebra/zapi_msg.c:2642
    #5 0x4e6d32 in zserv_process_messages zebra/zserv.c:517
    #6 0x7f08e3901995 in thread_call lib/thread.c:1549
    #7 0x7f08e38937d7 in frr_run lib/libfrr.c:1093
    #8 0x41686e in main zebra/main.c:470
    #9 0x7f08e2fe682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 32832 byte(s) leaked in 2 allocation(s).

This commit fixes these two different leaks.

Fixes: FRRouting#5658
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 15, 2020
The only two safi's that are usable for zebra for installation
of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST.
The acceptance of other safi's is causing a memory leak:

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2)
    #1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27
    #2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11
    #3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10
    #4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8
    #5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2
Collapse

Sequence of events:

Upon vrf creation there is a zvrf->table[afi][safi] data structure
that tables are auto created for.  These tables only create SAFI_UNICAST
and SAFI_MULTICAST tables.  Since these are the only safi types that
are zebra can actually work on.  zvrf data structures also have a
zvrf->otable data structure that tracks in a RB tree other tables
that are created ( say you have routes stuck in any random table
in the 32bit route table space in linux ).  This data structure is
only used if the lookup in zvrf->table[afi][safi] fails.

After creation if we pass a route down from an upper level protocol
that has non unicast or multicast safi *but* has the actual
tableid of the vrf we are in, the initial lookup will always
return NULL leaving us to look in the otable.  This will create
a data structure to track this data.

If after this event you pass in a second route with the same
afi/safi/table_id, the otable will be created and attempted
to be stored, but the RB_TREE_UNIQ data structure when it sees
this will return the original otable returned and the lookup function
zebra_vrf_get_table_with_table_id will just drop the second otable.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 15, 2020
The only two safi's that are usable for zebra for installation
of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST.
The acceptance of other safi's is causing a memory leak:

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2)
    #1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27
    #2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11
    #3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10
    #4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8
    #5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2
Collapse

Sequence of events:

Upon vrf creation there is a zvrf->table[afi][safi] data structure
that tables are auto created for.  These tables only create SAFI_UNICAST
and SAFI_MULTICAST tables.  Since these are the only safi types that
are zebra can actually work on.  zvrf data structures also have a
zvrf->otable data structure that tracks in a RB tree other tables
that are created ( say you have routes stuck in any random table
in the 32bit route table space in linux ).  This data structure is
only used if the lookup in zvrf->table[afi][safi] fails.

After creation if we pass a route down from an upper level protocol
that has non unicast or multicast safi *but* has the actual
tableid of the vrf we are in, the initial lookup will always
return NULL leaving us to look in the otable.  This will create
a data structure to track this data.

If after this event you pass in a second route with the same
afi/safi/table_id, the otable will be created and attempted
to be stored, but the RB_TREE_UNIQ data structure when it sees
this will return the original otable returned and the lookup function
zebra_vrf_get_table_with_table_id will just drop the second otable.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 15, 2020
==25402==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x533302 in calloc (/usr/lib/frr/zebra+0x533302)
    #1 0x7fee84cdc80b in qcalloc /home/qlyoung/frr/lib/memory.c:110:27
    #2 0x5a3032 in create_label_chunk /home/qlyoung/frr/zebra/label_manager.c:188:3
    #3 0x5a3c2b in assign_label_chunk /home/qlyoung/frr/zebra/label_manager.c:354:8
    #4 0x5a2a38 in label_manager_get_chunk /home/qlyoung/frr/zebra/label_manager.c:424:9
    #5 0x5a1412 in hook_call_lm_get_chunk /home/qlyoung/frr/zebra/label_manager.c:60:1
    #6 0x5a1412 in lm_get_chunk_call /home/qlyoung/frr/zebra/label_manager.c:81:2
    #7 0x72a234 in zread_get_label_chunk /home/qlyoung/frr/zebra/zapi_msg.c:2026:2
    #8 0x72a234 in zread_label_manager_request /home/qlyoung/frr/zebra/zapi_msg.c:2073:4
    #9 0x73150c in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2688:2

When creating label chunk that has a specified base, we eventually are
calling assign_specific_label_chunk. This function finds the appropriate
list node and deletes it from the lbl_mgr.lc_list but since
the function uses list_delete_node() the deletion function that is
specified for lbl_mgr.lc_list is not called thus dropping the memory.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 15, 2020
==25402==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x533302 in calloc (/usr/lib/frr/zebra+0x533302)
    #1 0x7fee84cdc80b in qcalloc /home/qlyoung/frr/lib/memory.c:110:27
    #2 0x5a3032 in create_label_chunk /home/qlyoung/frr/zebra/label_manager.c:188:3
    #3 0x5a3c2b in assign_label_chunk /home/qlyoung/frr/zebra/label_manager.c:354:8
    #4 0x5a2a38 in label_manager_get_chunk /home/qlyoung/frr/zebra/label_manager.c:424:9
    #5 0x5a1412 in hook_call_lm_get_chunk /home/qlyoung/frr/zebra/label_manager.c:60:1
    #6 0x5a1412 in lm_get_chunk_call /home/qlyoung/frr/zebra/label_manager.c:81:2
    #7 0x72a234 in zread_get_label_chunk /home/qlyoung/frr/zebra/zapi_msg.c:2026:2
    #8 0x72a234 in zread_label_manager_request /home/qlyoung/frr/zebra/zapi_msg.c:2073:4
    #9 0x73150c in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2688:2

When creating label chunk that has a specified base, we eventually are
calling assign_specific_label_chunk. This function finds the appropriate
list node and deletes it from the lbl_mgr.lc_list but since
the function uses list_delete_node() the deletion function that is
specified for lbl_mgr.lc_list is not called thus dropping the memory.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Jan 15, 2020
The only two safi's that are usable for zebra for installation
of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST.
The acceptance of other safi's is causing a memory leak:

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2)
    #1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27
    #2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11
    #3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10
    #4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8
    #5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2
Collapse

Sequence of events:

Upon vrf creation there is a zvrf->table[afi][safi] data structure
that tables are auto created for.  These tables only create SAFI_UNICAST
and SAFI_MULTICAST tables.  Since these are the only safi types that
are zebra can actually work on.  zvrf data structures also have a
zvrf->otable data structure that tracks in a RB tree other tables
that are created ( say you have routes stuck in any random table
in the 32bit route table space in linux ).  This data structure is
only used if the lookup in zvrf->table[afi][safi] fails.

After creation if we pass a route down from an upper level protocol
that has non unicast or multicast safi *but* has the actual
tableid of the vrf we are in, the initial lookup will always
return NULL leaving us to look in the otable.  This will create
a data structure to track this data.

If after this event you pass in a second route with the same
afi/safi/table_id, the otable will be created and attempted
to be stored, but the RB_TREE_UNIQ data structure when it sees
this will return the original otable returned and the lookup function
zebra_vrf_get_table_with_table_id will just drop the second otable.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp added a commit that referenced this pull request Mar 8, 2020
Upper level clients ask for default routes of a particular family
This change ensures that they only receive the family that they
have asked for.

Discovered when testing in ospf `default-information originate`

=================================================================
==246306==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffa2e8 at pc 0x7ffff73c44e2 bp 0x7fffffffa090 sp 0x7fffffffa088
READ of size 16 at 0x7fffffffa2e8 thread T0
    #0 0x7ffff73c44e1 in prefix_copy lib/prefix.c:310
    #1 0x7ffff741c0aa in route_node_lookup lib/table.c:255
    #2 0x5555556cd263 in ospf_external_info_delete ospfd/ospf_asbr.c:178
    #3 0x5555556a47cc in ospf_zebra_read_route ospfd/ospf_zebra.c:852
    #4 0x7ffff746f5d8 in zclient_read lib/zclient.c:3028
    #5 0x7ffff742fc91 in thread_call lib/thread.c:1549
    #6 0x7ffff7374642 in frr_run lib/libfrr.c:1093
    #7 0x5555555bfaef in main ospfd/ospf_main.c:235
    #8 0x7ffff70a2bba in __libc_start_main ../csu/libc-start.c:308
    #9 0x5555555bf499 in _start (/usr/lib/frr/ospfd+0x6b499)

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Mar 30, 2020
Implement tests to verify BGP link-bandwidth and weighted ECMP
functionality. These tests validate one of the primary use cases for
weighted ECMP (a.k.a. Unequal cost multipath) using BGP link-bandwidth:
https://tools.ietf.org/html/draft-mohanty-bess-ebgp-dmz

The included tests are:
Test #1: Test BGP link-bandwidth advertisement based on number of multipaths
Test #2: Test cumulative link-bandwidth propagation
Test #3: Test weighted ECMP - multipath with next hop weights
Test #4: Test weighted ECMP rebalancing upon change (link flap)
Test #5: Test weighted ECMP for a second anycast IP
Test #6: Test paths with and without link-bandwidth - receiver should resort to regular ECMP
Test #7: Test different options for processing link-bandwidth on the receiver

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Apr 8, 2020
Implement tests to verify BGP link-bandwidth and weighted ECMP
functionality. These tests validate one of the primary use cases for
weighted ECMP (a.k.a. Unequal cost multipath) using BGP link-bandwidth:
https://tools.ietf.org/html/draft-mohanty-bess-ebgp-dmz

The included tests are:
Test #1: Test BGP link-bandwidth advertisement based on number of multipaths
Test #2: Test cumulative link-bandwidth propagation
Test #3: Test weighted ECMP - multipath with next hop weights
Test #4: Test weighted ECMP rebalancing upon change (link flap)
Test #5: Test weighted ECMP for a second anycast IP
Test #6: Test paths with and without link-bandwidth - receiver should resort to regular ECMP
Test #7: Test different options for processing link-bandwidth on the receiver

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request May 7, 2020
Sample output -
root@torm-11:mgmt:~# net show bgp l2vpn evpn route vni 1000 mac 00:00:00:00:00:11
BGP routing table entry for [2]:[0]:[48]:[00:00:00:00:00:11]
Paths: (5 available, best #5)
  Not advertised to any peer
  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000
  Imported from 27.0.0.16:14:[2]:[0]:[48]:[00:00:00:00:00:11], VNI 1000
  4435 5551
    27.0.0.16 from spine-2(swp4) (27.0.0.14)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es
      Origin IGP, valid, external
      Extended Community: RT:5551:1000 RT:5551:4001 ET:8 Rmac:00:02:00:00:00:2d
      Last update: Fri Mar 27 02:26:35 2020

>>>>>>>>>>>>>>>>>>>> SNIP >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000/4001
  Local
    27.0.0.15 from 0.0.0.0 (27.0.0.15)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es peer-info: (active MM: 0) >>>
      Origin IGP, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (EVPN local ES path)
      Extended Community: ET:8 RT:5550:1000 RT:5550:4001 Rmac:00:02:00:00:00:25
      Last update: Fri Mar 27 02:26:35 2020

Displayed 5 paths for requested prefix
root@torm-11:mgmt:~#

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Jun 26, 2020
Sample output -
root@torm-11:mgmt:~# net show bgp l2vpn evpn route vni 1000 mac 00:00:00:00:00:11
BGP routing table entry for [2]:[0]:[48]:[00:00:00:00:00:11]
Paths: (5 available, best #5)
  Not advertised to any peer
  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000
  Imported from 27.0.0.16:14:[2]:[0]:[48]:[00:00:00:00:00:11], VNI 1000
  4435 5551
    27.0.0.16 from spine-2(swp4) (27.0.0.14)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es
      Origin IGP, valid, external
      Extended Community: RT:5551:1000 RT:5551:4001 ET:8 Rmac:00:02:00:00:00:2d
      Last update: Fri Mar 27 02:26:35 2020

>>>>>>>>>>>>>>>>>>>> SNIP >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000/4001
  Local
    27.0.0.15 from 0.0.0.0 (27.0.0.15)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es peer-info: (active MM: 0) >>>
      Origin IGP, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (EVPN local ES path)
      Extended Community: ET:8 RT:5550:1000 RT:5550:4001 Rmac:00:02:00:00:00:25
      Last update: Fri Mar 27 02:26:35 2020

Displayed 5 paths for requested prefix
root@torm-11:mgmt:~#

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Jun 26, 2020
=================================================================
==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>
donaldsharp pushed a commit that referenced this pull request Aug 6, 2020
Sample output -
root@torm-11:mgmt:~# net show bgp l2vpn evpn route vni 1000 mac 00:00:00:00:00:11
BGP routing table entry for [2]:[0]:[48]:[00:00:00:00:00:11]
Paths: (5 available, best #5)
  Not advertised to any peer
  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000
  Imported from 27.0.0.16:14:[2]:[0]:[48]:[00:00:00:00:00:11], VNI 1000
  4435 5551
    27.0.0.16 from spine-2(swp4) (27.0.0.14)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es
      Origin IGP, valid, external
      Extended Community: RT:5551:1000 RT:5551:4001 ET:8 Rmac:00:02:00:00:00:2d
      Last update: Fri Mar 27 02:26:35 2020

>>>>>>>>>>>>>>>>>>>> SNIP >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

  Route [2]:[0]:[48]:[00:00:00:00:00:11] VNI 1000/4001
  Local
    27.0.0.15 from 0.0.0.0 (27.0.0.15)
      ESI 03:00:00:00:00:01:11:00:00:01 local-es peer-info: (active MM: 0) >>>
      Origin IGP, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (EVPN local ES path)
      Extended Community: ET:8 RT:5550:1000 RT:5550:4001 Rmac:00:02:00:00:00:25
      Last update: Fri Mar 27 02:26:35 2020

Displayed 5 paths for requested prefix
root@torm-11:mgmt:~#

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
donaldsharp pushed a commit that referenced this pull request Nov 25, 2020
The fields in the broadcast/p2p union struct in an isis circuit are
initialized when the circuit goes up, but currently this step is
skipped if the interface is passive. This can create problems if the
circuit type (referred to as network type in the config) changes from
broadcast to point-to-point. We can end up with the p2p neighbor
pointer pointing at some garbage left by the broadcast struct in the
union, which would then cause a segfault the first time we would
dereference it - for example when building the lsp, or computing the
SPF tree.

compressed backtrace of a possible crash:
 #0  0x0000555555579a9c in lsp_build at frr/isisd/isis_lsp.c:1114
 #1  0x000055555557a516 in lsp_regenerate at frr/isisd/isis_lsp.c:1301
 #2  0x000055555557aa25 in lsp_refresh at frr/isisd/isis_lsp.c:1381
 #3  0x00007ffff7b2622c in thread_call at frr/lib/thread.c:1549
 #4  0x00007ffff7ad6df4 in frr_run at frr/lib/libfrr.c:1098
 #5  0x000055555556b67f in main at frr/isisd/isis_main.c:272

isis_lsp.c:
1112	case CIRCUIT_T_P2P: {
1113		struct isis_adjacency *nei = circuit->u.p2p.neighbor;
1114		if (nei && nei->adj_state == ISIS_ADJ_UP

Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
donaldsharp added a commit that referenced this pull request Nov 26, 2020
error	26-Nov-2020 14:35:02	ERROR: AddressSanitizer: heap-use-after-free on address 0x631000024838 at pc 0x55cefae977e9 bp 0x7ffdd3546860 sp 0x7ffdd3546850
error	26-Nov-2020 14:35:02	READ of size 4 at 0x631000024838 thread T0
error	26-Nov-2020 14:35:02	    #0 0x55cefae977e8 in ldpe_imsg_compose_parent_sync ldpd/ldpe.c:256
error	26-Nov-2020 14:35:02	    #1 0x55cefae9ab13 in vlog ldpd/log.c:53
error	26-Nov-2020 14:35:02	    #2 0x55cefae9b21f in log_info ldpd/log.c:102
error	26-Nov-2020 14:35:02	    #3 0x55cefae96eae in ldpe_shutdown ldpd/ldpe.c:237
error	26-Nov-2020 14:35:02	    #4 0x55cefae99254 in ldpe_dispatch_main ldpd/ldpe.c:585
error	26-Nov-2020 14:35:02	    #5 0x55cefaf93875 in thread_call lib/thread.c:1681
error	26-Nov-2020 14:35:02	    #6 0x55cefae97304 in ldpe ldpd/ldpe.c:136
error	26-Nov-2020 14:35:02	    #7 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #8 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	26-Nov-2020 14:35:02	    #9 0x55cefae525e9 in _start (/usr/lib/frr/ldpd+0xb35e9)
error	26-Nov-2020 14:35:02
error	26-Nov-2020 14:35:02	0x631000024838 is located 65592 bytes inside of 65632-byte region [0x631000014800,0x631000024860)
error	26-Nov-2020 14:35:02	freed by thread T0 here:
error	26-Nov-2020 14:35:02	    #0 0x7f4ef21e37a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
error	26-Nov-2020 14:35:02	    #1 0x55cefae96e91 in ldpe_shutdown ldpd/ldpe.c:234
error	26-Nov-2020 14:35:02	    #2 0x55cefae99254 in ldpe_dispatch_main ldpd/ldpe.c:585
error	26-Nov-2020 14:35:02	    #3 0x55cefaf93875 in thread_call lib/thread.c:1681
error	26-Nov-2020 14:35:02	    #4 0x55cefae97304 in ldpe ldpd/ldpe.c:136
error	26-Nov-2020 14:35:02	    #5 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #6 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	26-Nov-2020 14:35:02
error	26-Nov-2020 14:35:02	previously allocated by thread T0 here:
error	26-Nov-2020 14:35:02	    #0 0x7f4ef21e3d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
error	26-Nov-2020 14:35:02	    #1 0x55cefae9725d in ldpe ldpd/ldpe.c:127
error	26-Nov-2020 14:35:02	    #2 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #3 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Clean this problem up in the same way as the previous commit

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 19, 2020
We are using data after it has been freed and handed back to the
OS.
Address Sanitizer output:

error	23-Nov-2020 18:53:57	ERROR: AddressSanitizer: heap-use-after-free on address 0x631000024838 at pc 0x55f825998f58 bp 0x7fffa5b0f5b0 sp 0x7fffa5b0f5a0
error	23-Nov-2020 18:53:57	READ of size 4 at 0x631000024838 thread T0
error	23-Nov-2020 18:53:57	    #0 0x55f825998f57 in lde_imsg_compose_parent_sync ldpd/lde.c:226
error	23-Nov-2020 18:53:57	    #1 0x55f8259ca9ed in vlog ldpd/log.c:48
error	23-Nov-2020 18:53:57	    #2 0x55f8259cb1c8 in log_info ldpd/log.c:102
error	23-Nov-2020 18:53:57	    #3 0x55f82599e841 in lde_shutdown ldpd/lde.c:208
error	23-Nov-2020 18:53:57	    #4 0x55f8259a2703 in lde_dispatch_parent ldpd/lde.c:666
error	23-Nov-2020 18:53:57	    #5 0x55f825ac3815 in thread_call lib/thread.c:1681
error	23-Nov-2020 18:53:57	    #6 0x55f825998d5e in lde ldpd/lde.c:160
error	23-Nov-2020 18:53:57	    #7 0x55f82598a289 in main ldpd/ldpd.c:320
error	23-Nov-2020 18:53:57	    #8 0x7fe3f749db96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	23-Nov-2020 18:53:57	    #9 0x55f825982579 in _start (/usr/lib/frr/ldpd+0xb3579)
error	23-Nov-2020 18:53:57
error	23-Nov-2020 18:53:57	0x631000024838 is located 65592 bytes inside of 65632-byte region [0x631000014800,0x631000024860)
error	23-Nov-2020 18:53:57	freed by thread T0 here:
error	23-Nov-2020 18:53:57	    #0 0x7fe3f8a4d7a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
error	23-Nov-2020 18:53:57	    #1 0x55f82599e830 in lde_shutdown ldpd/lde.c:206
error	23-Nov-2020 18:53:57	    #2 0x55f8259a2703 in lde_dispatch_parent ldpd/lde.c:666
error	23-Nov-2020 18:53:57	    #3 0x55f825ac3815 in thread_call lib/thread.c:1681
error	23-Nov-2020 18:53:57	    #4 0x55f825998d5e in lde ldpd/lde.c:160
error	23-Nov-2020 18:53:57	    #5 0x55f82598a289 in main ldpd/ldpd.c:320
error	23-Nov-2020 18:53:57	    #6 0x7fe3f749db96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	23-Nov-2020 18:53:57
error	23-Nov-2020 18:53:57	previously allocated by thread T0 here:
error	23-Nov-2020 18:53:57	    #0 0x7fe3f8a4dd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
error	23-Nov-2020 18:53:57	    #1 0x55f825998cb7 in lde ldpd/lde.c:151
error	23-Nov-2020 18:53:57	    #2 0x55f82598a289 in main ldpd/ldpd.c:320
error	23-Nov-2020 18:53:57	    #3 0x7fe3f749db96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	23-Nov-2020 18:53:57

The fix is to put this in global space.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 19, 2020
error	26-Nov-2020 14:35:02	ERROR: AddressSanitizer: heap-use-after-free on address 0x631000024838 at pc 0x55cefae977e9 bp 0x7ffdd3546860 sp 0x7ffdd3546850
error	26-Nov-2020 14:35:02	READ of size 4 at 0x631000024838 thread T0
error	26-Nov-2020 14:35:02	    #0 0x55cefae977e8 in ldpe_imsg_compose_parent_sync ldpd/ldpe.c:256
error	26-Nov-2020 14:35:02	    #1 0x55cefae9ab13 in vlog ldpd/log.c:53
error	26-Nov-2020 14:35:02	    #2 0x55cefae9b21f in log_info ldpd/log.c:102
error	26-Nov-2020 14:35:02	    #3 0x55cefae96eae in ldpe_shutdown ldpd/ldpe.c:237
error	26-Nov-2020 14:35:02	    #4 0x55cefae99254 in ldpe_dispatch_main ldpd/ldpe.c:585
error	26-Nov-2020 14:35:02	    #5 0x55cefaf93875 in thread_call lib/thread.c:1681
error	26-Nov-2020 14:35:02	    #6 0x55cefae97304 in ldpe ldpd/ldpe.c:136
error	26-Nov-2020 14:35:02	    #7 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #8 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	26-Nov-2020 14:35:02	    #9 0x55cefae525e9 in _start (/usr/lib/frr/ldpd+0xb35e9)
error	26-Nov-2020 14:35:02
error	26-Nov-2020 14:35:02	0x631000024838 is located 65592 bytes inside of 65632-byte region [0x631000014800,0x631000024860)
error	26-Nov-2020 14:35:02	freed by thread T0 here:
error	26-Nov-2020 14:35:02	    #0 0x7f4ef21e37a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
error	26-Nov-2020 14:35:02	    #1 0x55cefae96e91 in ldpe_shutdown ldpd/ldpe.c:234
error	26-Nov-2020 14:35:02	    #2 0x55cefae99254 in ldpe_dispatch_main ldpd/ldpe.c:585
error	26-Nov-2020 14:35:02	    #3 0x55cefaf93875 in thread_call lib/thread.c:1681
error	26-Nov-2020 14:35:02	    #4 0x55cefae97304 in ldpe ldpd/ldpe.c:136
error	26-Nov-2020 14:35:02	    #5 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #6 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
error	26-Nov-2020 14:35:02
error	26-Nov-2020 14:35:02	previously allocated by thread T0 here:
error	26-Nov-2020 14:35:02	    #0 0x7f4ef21e3d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
error	26-Nov-2020 14:35:02	    #1 0x55cefae9725d in ldpe ldpd/ldpe.c:127
error	26-Nov-2020 14:35:02	    #2 0x55cefae5a2e2 in main ldpd/ldpd.c:322
error	26-Nov-2020 14:35:02	    #3 0x7f4ef0c33b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Clean this problem up in the same way as the previous commit

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Jan 21, 2021
When zebra is running with debugs turned on there
is a use after free reported by the address sanitizer:

2020/10/16 12:58:02 ZEBRA: rib_delnode: (0:254):4.5.6.16/32: rn 0x60b000026f20, re 0x6080000131a0, removing
2020/10/16 12:58:02 ZEBRA: rib_meta_queue_add: (0:254):4.5.6.16/32: queued rn 0x60b000026f20 into sub-queue 3
=================================================================
==3101430==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000011d28 at pc 0x555555705ab6 bp 0x7fffffffdab0 sp 0x7fffffffdaa8
READ of size 8 at 0x608000011d28 thread T0
    #0 0x555555705ab5 in re_list_const_first zebra/rib.h:222
    #1 0x555555705b54 in re_list_first zebra/rib.h:222
    #2 0x555555711a4f in process_subq_route zebra/zebra_rib.c:2248
    #3 0x555555711d2e in process_subq zebra/zebra_rib.c:2286
    #4 0x555555711ec7 in meta_queue_process zebra/zebra_rib.c:2320
    #5 0x7ffff74701f7 in work_queue_run lib/workqueue.c:291
    #6 0x7ffff7450e9c in thread_call lib/thread.c:1581
    #7 0x7ffff738eaf7 in frr_run lib/libfrr.c:1099
    #8 0x55555561a578 in main zebra/main.c:455
    #9 0x7ffff7079cc9 in __libc_start_main ../csu/libc-start.c:308
    #10 0x5555555e3429 in _start (/usr/lib/frr/zebra+0x8f429)
0x608000011d28 is located 8 bytes inside of 88-byte region [0x608000011d20,0x608000011d78)
freed by thread T0 here:
    #0 0x7ffff768bb6f in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.6+0xa9b6f)
    #1 0x7ffff739ccad in qfree lib/memory.c:129
    #2 0x555555709ee4 in rib_gc_dest zebra/zebra_rib.c:746
    #3 0x55555570ca76 in rib_process zebra/zebra_rib.c:1240
    #4 0x555555711a05 in process_subq_route zebra/zebra_rib.c:2245
    #5 0x555555711d2e in process_subq zebra/zebra_rib.c:2286
    #6 0x555555711ec7 in meta_queue_process zebra/zebra_rib.c:2320
    #7 0x7ffff74701f7 in work_queue_run lib/workqueue.c:291
    #8 0x7ffff7450e9c in thread_call lib/thread.c:1581
    #9 0x7ffff738eaf7 in frr_run lib/libfrr.c:1099
    #10 0x55555561a578 in main zebra/main.c:455
    #11 0x7ffff7079cc9 in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
    #0 0x7ffff768c037 in calloc (/lib/x86_64-linux-gnu/libasan.so.6+0xaa037)
    #1 0x7ffff739cb98 in qcalloc lib/memory.c:110
    #2 0x555555712ace in zebra_rib_create_dest zebra/zebra_rib.c:2515
    #3 0x555555712c6c in rib_link zebra/zebra_rib.c:2576
    #4 0x555555712faa in rib_addnode zebra/zebra_rib.c:2607
    #5 0x555555715bf0 in rib_add_multipath_nhe zebra/zebra_rib.c:3012
    #6 0x555555715f56 in rib_add_multipath zebra/zebra_rib.c:3049
    #7 0x55555571788b in rib_add zebra/zebra_rib.c:3327
    #8 0x5555555e584a in connected_up zebra/connected.c:254
    #9 0x5555555e42ff in connected_announce zebra/connected.c:94
    #10 0x5555555e4fd3 in connected_update zebra/connected.c:195
    #11 0x5555555e61ad in connected_add_ipv4 zebra/connected.c:340
    #12 0x5555555f26f5 in netlink_interface_addr zebra/if_netlink.c:1213
    #13 0x55555560f756 in netlink_information_fetch zebra/kernel_netlink.c:350
    #14 0x555555612e49 in netlink_parse_info zebra/kernel_netlink.c:941
    #15 0x55555560f9f1 in kernel_read zebra/kernel_netlink.c:402
    #16 0x7ffff7450e9c in thread_call lib/thread.c:1581
    #17 0x7ffff738eaf7 in frr_run lib/libfrr.c:1099
    #18 0x55555561a578 in main zebra/main.c:455
    #19 0x7ffff7079cc9 in __libc_start_main ../csu/libc-start.c:308
SUMMARY: AddressSanitizer: heap-use-after-free zebra/rib.h:222 in re_list_const_first

This is happening because we are using the dest pointer after a call into
rib_gc_dest.  In process_subq_route, we call rib_process() and if the
dest is deleted dest pointer is now garbage.  We must reload the
dest pointer in this case.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp pushed a commit that referenced this pull request Jul 7, 2021
Based in RFC 5440 @4.2.2 ""...after a no-path , the pcc may decide" and RFC
8231 #5.8.3 "... pcc must not set PcReq after path is delegated"

So will not (try) to delegate the path with no-path neither must do
further retries.

Signed-off-by: Javier Garcia <javier.garcia@voltanet.io>
donaldsharp pushed a commit that referenced this pull request Jul 7, 2021
Based on
RFC 8231 #5.8.3 PcUpd
RFC 8181 #5.1 Pcinit

Signed-off-by: Javier Garcia <javier.garcia@voltanet.io>
donaldsharp pushed a commit that referenced this pull request Mar 29, 2022
Fixing the crash:

> #0  0x0000560aa80f8e30 in lspdb_const_find (h=<error reading variable: Cannot access memory at address 0x7fff5e95efe8>, item=<error reading variable: Cannot access memory at address 0x7fff5e95efe0>) at ./isisd/isis_lsp.h:64
> #1  0x0000560aa80f8e9d in lspdb_find (h=0x560aaa1ed3b8, item=0x7fff5e95f050) at ./isisd/isis_lsp.h:64
> #2  0x0000560aa80f92f9 in lsp_search (head=0x560aaa1ed3b8, id=0x7fff5e95f200 "") at isisd/isis_lsp.c:100
> #3  0x0000560aa8113d69 in spf_adj_list_parse_tlv (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, id=0x560aad331a78 "", desig_is_id=0x0, pseudo_metric=0, metric=3, oldmetric=false, subtlvs=0x0) at isisd/isis_spf.c:1330
> #4  0x0000560aa811419d in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1429
> #5  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #6  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> (...)
> #65507 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65508 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65509 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65510 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65511 0x0000560aa8114313 in isis_spf_build_adj_list (spftree=0x560aaa1f09d0, lsp=0x560aaa1f4e50) at isisd/isis_spf.c:1455
> #65512 0x0000560aa8114f09 in isis_run_spf (spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1775
> #65513 0x0000560aa8115057 in isis_run_spf_with_protection (area=0x560aaa1ed3b0, spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1801
> #65514 0x0000560aa8115311 in isis_run_spf_cb (thread=0x7fff5f15e5a0) at isisd/isis_spf.c:1859
> #65515 0x00007f90bac66dcc in thread_call (thread=0x7fff5f15e5a0) at lib/thread.c:2002
> #65516 0x00007f90bac013ee in frr_run (master=0x560aa9f5cb40) at lib/libfrr.c:1196
> #65517 0x0000560aa80e7da2 in main (argc=2, argv=0x7fff5f15e7b8, envp=0x7fff5f15e7d0) at isisd/isis_main.c:273

Fixes: 7b36d36 ("isisd: make the SPF code more modular")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp pushed a commit that referenced this pull request Jul 5, 2022
Fixing the crash:

> #0  0x0000560aa80f8e30 in lspdb_const_find (h=<error reading variable: Cannot access memory at address 0x7fff5e95efe8>, item=<error reading variable: Cannot access memory at address 0x7fff5e95efe0>) at ./isisd/isis_lsp.h:64
> #1  0x0000560aa80f8e9d in lspdb_find (h=0x560aaa1ed3b8, item=0x7fff5e95f050) at ./isisd/isis_lsp.h:64
> #2  0x0000560aa80f92f9 in lsp_search (head=0x560aaa1ed3b8, id=0x7fff5e95f200 "") at isisd/isis_lsp.c:100
> #3  0x0000560aa8113d69 in spf_adj_list_parse_tlv (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, id=0x560aad331a78 "", desig_is_id=0x0, pseudo_metric=0, metric=3, oldmetric=false, subtlvs=0x0) at isisd/isis_spf.c:1330
> #4  0x0000560aa811419d in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1429
> #5  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #6  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> (...)
> #65507 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65508 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65509 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65510 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65511 0x0000560aa8114313 in isis_spf_build_adj_list (spftree=0x560aaa1f09d0, lsp=0x560aaa1f4e50) at isisd/isis_spf.c:1455
> #65512 0x0000560aa8114f09 in isis_run_spf (spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1775
> #65513 0x0000560aa8115057 in isis_run_spf_with_protection (area=0x560aaa1ed3b0, spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1801
> #65514 0x0000560aa8115311 in isis_run_spf_cb (thread=0x7fff5f15e5a0) at isisd/isis_spf.c:1859
> #65515 0x00007f90bac66dcc in thread_call (thread=0x7fff5f15e5a0) at lib/thread.c:2002
> #65516 0x00007f90bac013ee in frr_run (master=0x560aa9f5cb40) at lib/libfrr.c:1196
> #65517 0x0000560aa80e7da2 in main (argc=2, argv=0x7fff5f15e7b8, envp=0x7fff5f15e7d0) at isisd/isis_main.c:273

The fix is similar to the crash fix included in d9884a7
("isisd: Prepare IS-IS for Link State support"). The fix was:

> diff --git a/isisd/isis_lsp.c b/isisd/isis_lsp.c
> index 94353a5..92d329f 100644
> --- a/isisd/isis_lsp.c
> +++ b/isisd/isis_lsp.c
> @@ -2166,7 +2178,7 @@ int isis_lsp_iterate_ip_reach(struct isis_lsp *lsp, int family, uint16_t mtid,
>  	if (lsp->hdr.seqno == 0 || lsp->hdr.rem_lifetime == 0)
>  		return LSP_ITER_CONTINUE;
>
> -	/* Parse main LSP. */
> +	/* Parse LSP */
>  	if (lsp->tlvs) {
>  		if (!fabricd && !pseudo_lsp && family == AF_INET
>  		    && mtid == ISIS_MT_IPV4_UNICAST) {
> @@ -2236,13 +2248,17 @@ int isis_lsp_iterate_ip_reach(struct isis_lsp *lsp, int family, uint16_t mtid,
>  		}
>  	}
>
> -	/* Parse LSP fragments. */
> -	for (ALL_LIST_ELEMENTS_RO(lsp->lspu.frags, node, frag)) {
> -		if (!frag->tlvs)
> -			continue;
> +	/* Parse LSP fragments if it is not a fragment itself */
> +	if (!LSP_FRAGMENT(lsp->hdr.lsp_id))
> +		for (ALL_LIST_ELEMENTS_RO(lsp->lspu.frags, node, frag)) {
> +			if (!frag->tlvs)
> +				continue;
>
> -		isis_lsp_iterate_ip_reach(frag, family, mtid, cb, arg);
> -	}
> +			if (isis_lsp_iterate_ip_reach(frag, family, mtid, cb,
> +						      arg)
> +			    == LSP_ITER_STOP)
> +				return LSP_ITER_STOP;
> +		}
>
>  	return LSP_ITER_CONTINUE;
>  }

Fixes: 7b36d36 ("isisd: make the SPF code more modular")
Fixes: 5e56a50 ("isisd: fix infinite loop when parsing LSPs")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp added a commit that referenced this pull request Nov 29, 2022
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Nov 29, 2022
When deleting a unnumbered peer *and* in the process the LL address
is being removed there exists a case where the old peer data structure
would be left on the bnc->nht_info pointer.  This leads to cases where
when the nexthop resolution is also changing that there are use after
free operations.

Address Sanitizer that lead me to this:

=================================================================
==1018==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a000480480 at pc 0x556ded4d78c1 bp 0x7ffef0306fb0 sp 0x7ffef0306fa8
READ of size 8 at 0x62a000480480 thread T0
    #0 0x556ded4d78c0 in bgp_parse_nexthop_update bgpd/bgp_nht.c:431
    #1 0x556ded5b296a in bgp_read_nexthop_update bgpd/bgp_zebra.c:105
    #2 0x7f2c9be1aa85 in zclient_read lib/zclient.c:3570
    #3 0x7f2c9bde766d in thread_call lib/thread.c:1585
    #4 0x7f2c9bd634e7 in frr_run lib/libfrr.c:1123
    #5 0x556ded409a15 in main bgpd/bgp_main.c:540
    #6 0x7f2c9b81d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #7 0x556ded40c7f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a000480480 is located 640 bytes inside of 23376-byte region [0x62a000480200,0x62a000485d50)
freed by thread T0 here:
    #0 0x7f2c9c026fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x556ded5d3e42 in peer_free bgpd/bgpd.c:1113
    #2 0x556ded5d3e42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x556ded5d492e in peer_delete bgpd/bgpd.c:2457
    #4 0x556ded569e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f2c9bd0a160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f2c9bd0e112 in cmd_execute_command lib/command.c:1009
    #7 0x7f2c9bd0e573 in cmd_execute lib/command.c:1162
    #8 0x7f2c9bdf4402 in vty_command lib/vty.c:526
    #9 0x7f2c9bdf4832 in vty_execute lib/vty.c:1291
    #10 0x7f2c9bdfb741 in vtysh_read lib/vty.c:2130
    #11 0x7f2c9bde766d in thread_call lib/thread.c:1585
    #12 0x7f2c9bd634e7 in frr_run lib/libfrr.c:1123
    #13 0x556ded409a15 in main bgpd/bgp_main.c:540
    #14 0x7f2c9b81d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Nov 29, 2022
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 2, 2022
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 3, 2022
Config data was being freed just prior to it
being used for cleanup in shutdown.  Prevent this
from happening.

./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-=================================================================
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142:==2274142==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d00000c880 at pc 0x0000004d94d1 bp 0x7ffd46637810 sp 0
x7ffd46637808
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-READ of size 4 at 0x61d00000c880 thread T0
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4d94d0 in ldp_rtr_id_get /home/sharpd/frr8/ldpd/ldpd.c:983:20
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x56ff92 in gen_ldp_hdr /home/sharpd/frr8/ldpd/packet.c:47:19
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x56a4b0 in send_notification_full /home/sharpd/frr8/ldpd/notification.c:49:9
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #3 0x56c4b3 in send_notification /home/sharpd/frr8/ldpd/notification.c:117:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x573fb7 in session_shutdown /home/sharpd/frr8/ldpd/packet.c:666:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #5 0x4e2ef1 in adj_del /home/sharpd/frr8/ldpd/adjacency.c:145:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #6 0x55d425 in ldpe_shutdown /home/sharpd/frr8/ldpd/ldpe.c:231:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #7 0x55a9a0 in ldpe_dispatch_main /home/sharpd/frr8/ldpd/ldpe.c:631:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #8 0x7f0c00c035e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #9 0x5586f2 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:138:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #10 0x4d46d2 in main /home/sharpd/frr8/ldpd/ldpd.c:339:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #11 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #12 0x429cb9 in _start (/usr/lib/frr/ldpd+0x429cb9)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-0x61d00000c880 is located 0 bytes inside of 2008-byte region [0x61d00000c880,0x61d00000d058)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-freed by thread T0 here:
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4a3aad in free (/usr/lib/frr/ldpd+0x4a3aad)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x4de6c8 in config_clear /home/sharpd/frr8/ldpd/ldpd.c:2001:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x55d12d in ldpe_shutdown /home/sharpd/frr8/ldpd/ldpe.c:211:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #3 0x55a9a0 in ldpe_dispatch_main /home/sharpd/frr8/ldpd/ldpe.c:631:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x7f0c00c035e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #5 0x5586f2 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:138:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #6 0x4d46d2 in main /home/sharpd/frr8/ldpd/ldpd.c:339:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #7 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-previously allocated by thread T0 here:
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4a3ea2 in calloc (/usr/lib/frr/ldpd+0x4a3ea2)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x4d6146 in config_new_empty /home/sharpd/frr8/ldpd/ldpd.c:1967:10
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x558678 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:134:11
--
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 3, 2022
On shutdown a use after free was being seen of a route table.
Basically the pointer was kept around and resent for cleanup.
Probably something needs to be unwound to make this better
in the future.  Just cleaning up the use after free.

./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-=================================================================
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929:==911929==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000127a00 at pc 0x7fb9ad546f5b bp 0x7ffc3cff0330 sp 0x7ffc3
cff0328
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-READ of size 8 at 0x606000127a00 thread T0
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #0 0x7fb9ad546f5a in route_table_free /home/sharpd/frr8/lib/table.c:103:13
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #1 0x7fb9ad546f04 in route_table_finish /home/sharpd/frr8/lib/table.c:61:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #2 0x6b94ba in zebra_ns_disable_internal /home/sharpd/frr8/zebra/zebra_ns.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #3 0x6b9158 in zebra_ns_disabled /home/sharpd/frr8/zebra/zebra_ns.c:116:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #4 0x7fb9ad43f0f5 in ns_disable_internal /home/sharpd/frr8/lib/netns_linux.c:273:4
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #5 0x7fb9ad43e634 in ns_disable /home/sharpd/frr8/lib/netns_linux.c:368:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #6 0x7fb9ad43e251 in ns_delete /home/sharpd/frr8/lib/netns_linux.c:330:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x7fb9ad43fbb3 in ns_terminate /home/sharpd/frr8/lib/netns_linux.c:524:3
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #8 0x54f8de in zebra_finalize /home/sharpd/frr8/zebra/main.c:232:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #9 0x7fb9ad5655e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #10 0x7fb9ad3d3343 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #11 0x550b48 in main /home/sharpd/frr8/zebra/main.c:476:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #12 0x7fb9acd30d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #13 0x443549 in _start (/usr/lib/frr/zebra+0x443549)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-0x606000127a00 is located 0 bytes inside of 56-byte region [0x606000127a00,0x606000127a38)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-freed by thread T0 here:
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #0 0x4bd33d in free (/usr/lib/frr/zebra+0x4bd33d)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #1 0x7fb9ad42cc80 in qfree /home/sharpd/frr8/lib/memory.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #2 0x7fb9ad547305 in route_table_free /home/sharpd/frr8/lib/table.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #3 0x7fb9ad546f04 in route_table_finish /home/sharpd/frr8/lib/table.c:61:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #4 0x6b94ba in zebra_ns_disable_internal /home/sharpd/frr8/zebra/zebra_ns.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #5 0x6b9692 in zebra_ns_early_shutdown /home/sharpd/frr8/zebra/zebra_ns.c:164:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #6 0x7fb9ad43f228 in ns_walk_func /home/sharpd/frr8/lib/netns_linux.c:386:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x55014f in sigint /home/sharpd/frr8/zebra/main.c:194:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #8 0x7fb9ad50db99 in frr_sigevent_process /home/sharpd/frr8/lib/sigevent.c:130:6
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #9 0x7fb9ad560d07 in thread_fetch /home/sharpd/frr8/lib/thread.c:1775:4
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #10 0x7fb9ad3d332d in frr_run /home/sharpd/frr8/lib/libfrr.c:1197:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #11 0x550b48 in main /home/sharpd/frr8/zebra/main.c:476:2
--
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x7fb9acd30d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 5, 2022
Config data was being freed just prior to it
being used for cleanup in shutdown.  Prevent this
from happening.

./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-=================================================================
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142:==2274142==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d00000c880 at pc 0x0000004d94d1 bp 0x7ffd46637810 sp 0
x7ffd46637808
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-READ of size 4 at 0x61d00000c880 thread T0
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4d94d0 in ldp_rtr_id_get /home/sharpd/frr8/ldpd/ldpd.c:983:20
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x56ff92 in gen_ldp_hdr /home/sharpd/frr8/ldpd/packet.c:47:19
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x56a4b0 in send_notification_full /home/sharpd/frr8/ldpd/notification.c:49:9
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #3 0x56c4b3 in send_notification /home/sharpd/frr8/ldpd/notification.c:117:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x573fb7 in session_shutdown /home/sharpd/frr8/ldpd/packet.c:666:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #5 0x4e2ef1 in adj_del /home/sharpd/frr8/ldpd/adjacency.c:145:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #6 0x55d425 in ldpe_shutdown /home/sharpd/frr8/ldpd/ldpe.c:231:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #7 0x55a9a0 in ldpe_dispatch_main /home/sharpd/frr8/ldpd/ldpe.c:631:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #8 0x7f0c00c035e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #9 0x5586f2 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:138:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #10 0x4d46d2 in main /home/sharpd/frr8/ldpd/ldpd.c:339:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #11 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #12 0x429cb9 in _start (/usr/lib/frr/ldpd+0x429cb9)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-0x61d00000c880 is located 0 bytes inside of 2008-byte region [0x61d00000c880,0x61d00000d058)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-freed by thread T0 here:
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4a3aad in free (/usr/lib/frr/ldpd+0x4a3aad)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x4de6c8 in config_clear /home/sharpd/frr8/ldpd/ldpd.c:2001:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x55d12d in ldpe_shutdown /home/sharpd/frr8/ldpd/ldpe.c:211:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #3 0x55a9a0 in ldpe_dispatch_main /home/sharpd/frr8/ldpd/ldpe.c:631:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x7f0c00c035e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #5 0x5586f2 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:138:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #6 0x4d46d2 in main /home/sharpd/frr8/ldpd/ldpd.c:339:3
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #7 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-previously allocated by thread T0 here:
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #0 0x4a3ea2 in calloc (/usr/lib/frr/ldpd+0x4a3ea2)
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #1 0x4d6146 in config_new_empty /home/sharpd/frr8/ldpd/ldpd.c:1967:10
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #2 0x558678 in ldpe /home/sharpd/frr8/ldpd/ldpe.c:134:11
--
./isis_rlfa_topo1.test_isis_rlfa_topo1/rt8.ldpd.asan.2274142-    #4 0x7f0c00476d09 in __libc_start_main csu/../csu/libc-start.c:308:16

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 5, 2022
On shutdown a use after free was being seen of a route table.
Basically the pointer was kept around and resent for cleanup.
Probably something needs to be unwound to make this better
in the future.  Just cleaning up the use after free.

./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-=================================================================
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929:==911929==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000127a00 at pc 0x7fb9ad546f5b bp 0x7ffc3cff0330 sp 0x7ffc3
cff0328
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-READ of size 8 at 0x606000127a00 thread T0
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #0 0x7fb9ad546f5a in route_table_free /home/sharpd/frr8/lib/table.c:103:13
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #1 0x7fb9ad546f04 in route_table_finish /home/sharpd/frr8/lib/table.c:61:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #2 0x6b94ba in zebra_ns_disable_internal /home/sharpd/frr8/zebra/zebra_ns.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #3 0x6b9158 in zebra_ns_disabled /home/sharpd/frr8/zebra/zebra_ns.c:116:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #4 0x7fb9ad43f0f5 in ns_disable_internal /home/sharpd/frr8/lib/netns_linux.c:273:4
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #5 0x7fb9ad43e634 in ns_disable /home/sharpd/frr8/lib/netns_linux.c:368:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #6 0x7fb9ad43e251 in ns_delete /home/sharpd/frr8/lib/netns_linux.c:330:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x7fb9ad43fbb3 in ns_terminate /home/sharpd/frr8/lib/netns_linux.c:524:3
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #8 0x54f8de in zebra_finalize /home/sharpd/frr8/zebra/main.c:232:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #9 0x7fb9ad5655e6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #10 0x7fb9ad3d3343 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #11 0x550b48 in main /home/sharpd/frr8/zebra/main.c:476:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #12 0x7fb9acd30d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #13 0x443549 in _start (/usr/lib/frr/zebra+0x443549)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-0x606000127a00 is located 0 bytes inside of 56-byte region [0x606000127a00,0x606000127a38)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-freed by thread T0 here:
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #0 0x4bd33d in free (/usr/lib/frr/zebra+0x4bd33d)
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #1 0x7fb9ad42cc80 in qfree /home/sharpd/frr8/lib/memory.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #2 0x7fb9ad547305 in route_table_free /home/sharpd/frr8/lib/table.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #3 0x7fb9ad546f04 in route_table_finish /home/sharpd/frr8/lib/table.c:61:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #4 0x6b94ba in zebra_ns_disable_internal /home/sharpd/frr8/zebra/zebra_ns.c:141:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #5 0x6b9692 in zebra_ns_early_shutdown /home/sharpd/frr8/zebra/zebra_ns.c:164:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #6 0x7fb9ad43f228 in ns_walk_func /home/sharpd/frr8/lib/netns_linux.c:386:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x55014f in sigint /home/sharpd/frr8/zebra/main.c:194:2
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #8 0x7fb9ad50db99 in frr_sigevent_process /home/sharpd/frr8/lib/sigevent.c:130:6
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #9 0x7fb9ad560d07 in thread_fetch /home/sharpd/frr8/lib/thread.c:1775:4
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #10 0x7fb9ad3d332d in frr_run /home/sharpd/frr8/lib/libfrr.c:1197:9
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #11 0x550b48 in main /home/sharpd/frr8/zebra/main.c:476:2
--
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-    #7 0x7fb9acd30d09 in __libc_start_main csu/../csu/libc-start.c:308:16
./bfd_vrf_topo1.test_bfd_vrf_topo1/r2.zebra.asan.911929-

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 5, 2022
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 14, 2022
Address Sanitizer found this:

=================================================================
==418623==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 128 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeaba40f4 in nexthop_group_new /home/sharpd/frr8/lib/nexthop_group.c:270:9
    #3 0x56859b in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:950:9
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

Indirect leak of 576 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeab9b3f8 in nexthop_new /home/sharpd/frr8/lib/nexthop.c:373:7
    #3 0x56875e in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:960:15
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: 704 byte(s) leaked in 8 allocation(s).

Fix this!

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 15, 2022
Address Sanitizer found this:

=================================================================
==418623==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 128 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeaba40f4 in nexthop_group_new /home/sharpd/frr8/lib/nexthop_group.c:270:9
    #3 0x56859b in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:950:9
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

Indirect leak of 576 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeab9b3f8 in nexthop_new /home/sharpd/frr8/lib/nexthop.c:373:7
    #3 0x56875e in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:960:15
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: 704 byte(s) leaked in 8 allocation(s).

Fix this!

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Dec 15, 2022
Address Sanitizer found this:

=================================================================
==418623==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 128 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeaba40f4 in nexthop_group_new /home/sharpd/frr8/lib/nexthop_group.c:270:9
    #3 0x56859b in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:950:9
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

Indirect leak of 576 byte(s) in 4 object(s) allocated from:
    #0 0x4bd732 in calloc (/usr/lib/frr/zebra+0x4bd732)
    #1 0x7feaeab8f798 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0x7feaeab9b3f8 in nexthop_new /home/sharpd/frr8/lib/nexthop.c:373:7
    #3 0x56875e in netlink_route_change_read_unicast /home/sharpd/frr8/zebra/rt_netlink.c:960:15
    #4 0x5651c2 in netlink_route_change /home/sharpd/frr8/zebra/rt_netlink.c:1204:2
    #5 0x54af15 in netlink_information_fetch /home/sharpd/frr8/zebra/kernel_netlink.c:407:10
    #6 0x53e7a3 in netlink_parse_info /home/sharpd/frr8/zebra/kernel_netlink.c:1184:12
    #7 0x548d46 in kernel_read /home/sharpd/frr8/zebra/kernel_netlink.c:501:2
    #8 0x7feaeacc87f6 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
    #9 0x7feaeab36503 in frr_run /home/sharpd/frr8/lib/libfrr.c:1198:3
    #10 0x550d38 in main /home/sharpd/frr8/zebra/main.c:476:2
    #11 0x7feaea492d09 in __libc_start_main csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: 704 byte(s) leaked in 8 allocation(s).

Fix this!

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp pushed a commit that referenced this pull request Dec 16, 2022
ASAN reported the following memleak:
```
Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x4d4342 in calloc (/usr/lib/frr/bgpd+0x4d4342)
    #1 0xbc3d68 in qcalloc /home/sharpd/frr8/lib/memory.c:116:27
    #2 0xb869f7 in list_new /home/sharpd/frr8/lib/linklist.c:64:9
    #3 0x5a38bc in bgp_evpn_remote_ip_hash_alloc /home/sharpd/frr8/bgpd/bgp_evpn.c:6789:24
    #4 0xb358d3 in hash_get /home/sharpd/frr8/lib/hash.c:162:13
    #5 0x593d39 in bgp_evpn_remote_ip_hash_add /home/sharpd/frr8/bgpd/bgp_evpn.c:6881:7
    #6 0x59dbbd in install_evpn_route_entry_in_vni_common /home/sharpd/frr8/bgpd/bgp_evpn.c:3049:2
    #7 0x59cfe0 in install_evpn_route_entry_in_vni_ip /home/sharpd/frr8/bgpd/bgp_evpn.c:3126:8
    #8 0x59c6f0 in install_evpn_route_entry /home/sharpd/frr8/bgpd/bgp_evpn.c:3318:8
    #9 0x59bb52 in install_uninstall_route_in_vnis /home/sharpd/frr8/bgpd/bgp_evpn.c:3888:10
    #10 0x59b6d2 in bgp_evpn_install_uninstall_table /home/sharpd/frr8/bgpd/bgp_evpn.c:4019:5
    #11 0x578857 in install_uninstall_evpn_route /home/sharpd/frr8/bgpd/bgp_evpn.c:4051:9
    #12 0x58ada6 in bgp_evpn_import_route /home/sharpd/frr8/bgpd/bgp_evpn.c:6049:9
    #13 0x713794 in bgp_update /home/sharpd/frr8/bgpd/bgp_route.c:4842:3
    #14 0x583fa0 in process_type2_route /home/sharpd/frr8/bgpd/bgp_evpn.c:4518:9
    #15 0x5824ba in bgp_nlri_parse_evpn /home/sharpd/frr8/bgpd/bgp_evpn.c:5732:8
    #16 0x6ae6a2 in bgp_nlri_parse /home/sharpd/frr8/bgpd/bgp_packet.c:363:10
    #17 0x6be6fa in bgp_update_receive /home/sharpd/frr8/bgpd/bgp_packet.c:2020:15
    #18 0x6b7433 in bgp_process_packet /home/sharpd/frr8/bgpd/bgp_packet.c:2929:11
    #19 0xd00146 in thread_call /home/sharpd/frr8/lib/thread.c:2006:2
```

The list itself was not being cleaned up when the final list entry was
removed, so make sure we do that instead of leaking memory.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
donaldsharp pushed a commit that referenced this pull request Jan 11, 2023
Fix crash on "show bgp all" when BGP EVPN is set.

> #0  raise (sig=11) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fdfe03cf53c in core_handler (signo=11, siginfo=0x7ffdebbffe30, context=0x7ffdebbffd00) at lib/sigevent.c:261
> #2  <signal handler called>
> #3  0x00000000004d4fec in bgp_attr_get_community (attr=0x41) at bgpd/bgp_attr.h:553
> #4  0x00000000004eee84 in bgp_show_table (vty=0x1a790d0, bgp=0x19d0a00, safi=SAFI_EVPN, table=0x19f6010, type=bgp_show_type_normal, output_arg=0x0, rd=0x0, is_last=1, output_cum=0x0,
>     total_cum=0x0, json_header_depth=0x7ffdebc00bf8, show_flags=4, rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11329
> #5  0x00000000004f7765 in bgp_show (vty=0x1a790d0, bgp=0x19d0a00, afi=AFI_L2VPN, safi=SAFI_EVPN, type=bgp_show_type_normal, output_arg=0x0, show_flags=4,
>     rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11814
> #6  0x00000000004fb53b in show_ip_bgp_magic (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050, viewvrfname=0x0, all=0x1395390 "all", aa_nn=0x0, community_list=0,
>     community_list_str=0x0, community_list_name=0x0, as_path_filter_name=0x0, prefix_list=0x0, accesslist_name=0x0, rmap_name=0x0, version=0, version_str=0x0, alias_name=0x0,
>     orr_group_name=0x0, detail_routes=0x0, uj=0x0, detail_json=0x0, wide=0x0) at bgpd/bgp_route.c:13040
> #7  0x00000000004fa322 in show_ip_bgp (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050) at ./bgpd/bgp_route_clippy.c:519
> #8  0x00007fdfe033ccc8 in cmd_execute_command_real (vline=0x19c9300, filter=FILTER_RELAXED, vty=0x1a790d0, cmd=0x0, up_level=0) at lib/command.c:996
> #9  0x00007fdfe033c739 in cmd_execute_command (vline=0x19c9300, vty=0x1a790d0, cmd=0x0, vtysh=0) at lib/command.c:1056
> #10 0x00007fdfe033cdf5 in cmd_execute (vty=0x1a790d0, cmd=0x19c9eb0 "show bgp all", matched=0x0, vtysh=0) at lib/command.c:1223
> #11 0x00007fdfe03f65c6 in vty_command (vty=0x1a790d0, buf=0x19c9eb0 "show bgp all") at lib/vty.c:486
> #12 0x00007fdfe03f603b in vty_execute (vty=0x1a790d0) at lib/vty.c:1249
> #13 0x00007fdfe03f533b in vtysh_read (thread=0x7ffdebc03838) at lib/vty.c:2148
> #14 0x00007fdfe03e815d in thread_call (thread=0x7ffdebc03838) at lib/thread.c:2006
> #15 0x00007fdfe0379b54 in frr_run (master=0x1246880) at lib/libfrr.c:1198
> #16 0x000000000042b2a8 in main (argc=7, argv=0x7ffdebc03af8) at bgpd/bgp_main.c:520

Link: FRRouting#12576
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp pushed a commit that referenced this pull request Feb 3, 2023
Fixing the crash:

> #0  0x0000560aa80f8e30 in lspdb_const_find (h=<error reading variable: Cannot access memory at address 0x7fff5e95efe8>, item=<error reading variable: Cannot access memory at address 0x7fff5e95efe0>) at ./isisd/isis_lsp.h:64
> #1  0x0000560aa80f8e9d in lspdb_find (h=0x560aaa1ed3b8, item=0x7fff5e95f050) at ./isisd/isis_lsp.h:64
> #2  0x0000560aa80f92f9 in lsp_search (head=0x560aaa1ed3b8, id=0x7fff5e95f200 "") at isisd/isis_lsp.c:100
> #3  0x0000560aa8113d69 in spf_adj_list_parse_tlv (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, id=0x560aad331a78 "", desig_is_id=0x0, pseudo_metric=0, metric=3, oldmetric=false, subtlvs=0x0) at isisd/isis_spf.c:1330
> #4  0x0000560aa811419d in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1429
> #5  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #6  0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> (...)
> #65507 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65508 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65509 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1ff8e0, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65510 0x0000560aa81141fe in spf_adj_list_parse_lsp (spftree=0x560aaa1f09d0, adj_list=0x560aaa214480, lsp=0x560aaa1f4e50, pseudo_nodeid=0x0, pseudo_metric=0) at isisd/isis_spf.c:1442
> #65511 0x0000560aa8114313 in isis_spf_build_adj_list (spftree=0x560aaa1f09d0, lsp=0x560aaa1f4e50) at isisd/isis_spf.c:1455
> #65512 0x0000560aa8114f09 in isis_run_spf (spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1775
> #65513 0x0000560aa8115057 in isis_run_spf_with_protection (area=0x560aaa1ed3b0, spftree=0x560aaa1f09d0) at isisd/isis_spf.c:1801
> #65514 0x0000560aa8115311 in isis_run_spf_cb (thread=0x7fff5f15e5a0) at isisd/isis_spf.c:1859
> #65515 0x00007f90bac66dcc in thread_call (thread=0x7fff5f15e5a0) at lib/thread.c:2002
> #65516 0x00007f90bac013ee in frr_run (master=0x560aa9f5cb40) at lib/libfrr.c:1196
> #65517 0x0000560aa80e7da2 in main (argc=2, argv=0x7fff5f15e7b8, envp=0x7fff5f15e7d0) at isisd/isis_main.c:273

The fix is similar to the crash fix included in d9884a7
("isisd: Prepare IS-IS for Link State support"). The fix was:

> diff --git a/isisd/isis_lsp.c b/isisd/isis_lsp.c
> index 94353a5..92d329f 100644
> --- a/isisd/isis_lsp.c
> +++ b/isisd/isis_lsp.c
> @@ -2166,7 +2178,7 @@ int isis_lsp_iterate_ip_reach(struct isis_lsp *lsp, int family, uint16_t mtid,
>  	if (lsp->hdr.seqno == 0 || lsp->hdr.rem_lifetime == 0)
>  		return LSP_ITER_CONTINUE;
>
> -	/* Parse main LSP. */
> +	/* Parse LSP */
>  	if (lsp->tlvs) {
>  		if (!fabricd && !pseudo_lsp && family == AF_INET
>  		    && mtid == ISIS_MT_IPV4_UNICAST) {
> @@ -2236,13 +2248,17 @@ int isis_lsp_iterate_ip_reach(struct isis_lsp *lsp, int family, uint16_t mtid,
>  		}
>  	}
>
> -	/* Parse LSP fragments. */
> -	for (ALL_LIST_ELEMENTS_RO(lsp->lspu.frags, node, frag)) {
> -		if (!frag->tlvs)
> -			continue;
> +	/* Parse LSP fragments if it is not a fragment itself */
> +	if (!LSP_FRAGMENT(lsp->hdr.lsp_id))
> +		for (ALL_LIST_ELEMENTS_RO(lsp->lspu.frags, node, frag)) {
> +			if (!frag->tlvs)
> +				continue;
>
> -		isis_lsp_iterate_ip_reach(frag, family, mtid, cb, arg);
> -	}
> +			if (isis_lsp_iterate_ip_reach(frag, family, mtid, cb,
> +						      arg)
> +			    == LSP_ITER_STOP)
> +				return LSP_ITER_STOP;
> +		}
>
>  	return LSP_ITER_CONTINUE;
>  }

Fixes: 7b36d36 ("isisd: make the SPF code more modular")
Fixes: 5e56a50 ("isisd: fix infinite loop when parsing LSPs")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 8c8a5a0)
donaldsharp added a commit that referenced this pull request Feb 22, 2023
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp added a commit that referenced this pull request Feb 23, 2023
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp pushed a commit that referenced this pull request Feb 23, 2023
Fix crash on "show bgp all" when BGP EVPN is set.

> #0  raise (sig=11) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fdfe03cf53c in core_handler (signo=11, siginfo=0x7ffdebbffe30, context=0x7ffdebbffd00) at lib/sigevent.c:261
> #2  <signal handler called>
> #3  0x00000000004d4fec in bgp_attr_get_community (attr=0x41) at bgpd/bgp_attr.h:553
> #4  0x00000000004eee84 in bgp_show_table (vty=0x1a790d0, bgp=0x19d0a00, safi=SAFI_EVPN, table=0x19f6010, type=bgp_show_type_normal, output_arg=0x0, rd=0x0, is_last=1, output_cum=0x0,
>     total_cum=0x0, json_header_depth=0x7ffdebc00bf8, show_flags=4, rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11329
> #5  0x00000000004f7765 in bgp_show (vty=0x1a790d0, bgp=0x19d0a00, afi=AFI_L2VPN, safi=SAFI_EVPN, type=bgp_show_type_normal, output_arg=0x0, show_flags=4,
>     rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11814
> #6  0x00000000004fb53b in show_ip_bgp_magic (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050, viewvrfname=0x0, all=0x1395390 "all", aa_nn=0x0, community_list=0,
>     community_list_str=0x0, community_list_name=0x0, as_path_filter_name=0x0, prefix_list=0x0, accesslist_name=0x0, rmap_name=0x0, version=0, version_str=0x0, alias_name=0x0,
>     orr_group_name=0x0, detail_routes=0x0, uj=0x0, detail_json=0x0, wide=0x0) at bgpd/bgp_route.c:13040
> #7  0x00000000004fa322 in show_ip_bgp (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050) at ./bgpd/bgp_route_clippy.c:519
> #8  0x00007fdfe033ccc8 in cmd_execute_command_real (vline=0x19c9300, filter=FILTER_RELAXED, vty=0x1a790d0, cmd=0x0, up_level=0) at lib/command.c:996
> #9  0x00007fdfe033c739 in cmd_execute_command (vline=0x19c9300, vty=0x1a790d0, cmd=0x0, vtysh=0) at lib/command.c:1056
> #10 0x00007fdfe033cdf5 in cmd_execute (vty=0x1a790d0, cmd=0x19c9eb0 "show bgp all", matched=0x0, vtysh=0) at lib/command.c:1223
> #11 0x00007fdfe03f65c6 in vty_command (vty=0x1a790d0, buf=0x19c9eb0 "show bgp all") at lib/vty.c:486
> #12 0x00007fdfe03f603b in vty_execute (vty=0x1a790d0) at lib/vty.c:1249
> #13 0x00007fdfe03f533b in vtysh_read (thread=0x7ffdebc03838) at lib/vty.c:2148
> #14 0x00007fdfe03e815d in thread_call (thread=0x7ffdebc03838) at lib/thread.c:2006
> #15 0x00007fdfe0379b54 in frr_run (master=0x1246880) at lib/libfrr.c:1198
> #16 0x000000000042b2a8 in main (argc=7, argv=0x7ffdebc03af8) at bgpd/bgp_main.c:520

Link: FRRouting#12576
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp added a commit that referenced this pull request Feb 24, 2023
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
donaldsharp pushed a commit that referenced this pull request Feb 24, 2023
Fix crash on "show bgp all" when BGP EVPN is set.

> #0  raise (sig=11) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fdfe03cf53c in core_handler (signo=11, siginfo=0x7ffdebbffe30, context=0x7ffdebbffd00) at lib/sigevent.c:261
> #2  <signal handler called>
> #3  0x00000000004d4fec in bgp_attr_get_community (attr=0x41) at bgpd/bgp_attr.h:553
> #4  0x00000000004eee84 in bgp_show_table (vty=0x1a790d0, bgp=0x19d0a00, safi=SAFI_EVPN, table=0x19f6010, type=bgp_show_type_normal, output_arg=0x0, rd=0x0, is_last=1, output_cum=0x0,
>     total_cum=0x0, json_header_depth=0x7ffdebc00bf8, show_flags=4, rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11329
> #5  0x00000000004f7765 in bgp_show (vty=0x1a790d0, bgp=0x19d0a00, afi=AFI_L2VPN, safi=SAFI_EVPN, type=bgp_show_type_normal, output_arg=0x0, show_flags=4,
>     rpki_target_state=RPKI_NOT_BEING_USED) at bgpd/bgp_route.c:11814
> #6  0x00000000004fb53b in show_ip_bgp_magic (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050, viewvrfname=0x0, all=0x1395390 "all", aa_nn=0x0, community_list=0,
>     community_list_str=0x0, community_list_name=0x0, as_path_filter_name=0x0, prefix_list=0x0, accesslist_name=0x0, rmap_name=0x0, version=0, version_str=0x0, alias_name=0x0,
>     orr_group_name=0x0, detail_routes=0x0, uj=0x0, detail_json=0x0, wide=0x0) at bgpd/bgp_route.c:13040
> #7  0x00000000004fa322 in show_ip_bgp (self=0x6752b0 <show_ip_bgp_cmd>, vty=0x1a790d0, argc=3, argv=0x19cb050) at ./bgpd/bgp_route_clippy.c:519
> #8  0x00007fdfe033ccc8 in cmd_execute_command_real (vline=0x19c9300, filter=FILTER_RELAXED, vty=0x1a790d0, cmd=0x0, up_level=0) at lib/command.c:996
> #9  0x00007fdfe033c739 in cmd_execute_command (vline=0x19c9300, vty=0x1a790d0, cmd=0x0, vtysh=0) at lib/command.c:1056
> #10 0x00007fdfe033cdf5 in cmd_execute (vty=0x1a790d0, cmd=0x19c9eb0 "show bgp all", matched=0x0, vtysh=0) at lib/command.c:1223
> #11 0x00007fdfe03f65c6 in vty_command (vty=0x1a790d0, buf=0x19c9eb0 "show bgp all") at lib/vty.c:486
> #12 0x00007fdfe03f603b in vty_execute (vty=0x1a790d0) at lib/vty.c:1249
> #13 0x00007fdfe03f533b in vtysh_read (thread=0x7ffdebc03838) at lib/vty.c:2148
> #14 0x00007fdfe03e815d in thread_call (thread=0x7ffdebc03838) at lib/thread.c:2006
> #15 0x00007fdfe0379b54 in frr_run (master=0x1246880) at lib/libfrr.c:1198
> #16 0x000000000042b2a8 in main (argc=7, argv=0x7ffdebc03af8) at bgpd/bgp_main.c:520

Link: FRRouting#12576
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp added a commit that referenced this pull request Feb 24, 2023
When changing the peers sockunion structure the bgp->peer
list was not being updated properly.  Since the peer's su
is being used for a sorted insert then the change of it requires
that the value be pulled out of the bgp->peer list and then
put back into as well.

Additionally ensure that the hash is always released on peer
deletion.

Lead to this from this decode in a address sanitizer run.

=================================================================
==30778==ERROR: AddressSanitizer: heap-use-after-free on address 0x62a0000d8440 at pc 0x7f48c9c5c547 bp 0x7ffcba272cb0 sp 0x7ffcba272ca8
READ of size 2 at 0x62a0000d8440 thread T0
    #0 0x7f48c9c5c546 in sockunion_same lib/sockunion.c:425
    #1 0x55cfefe3000f in peer_hash_same bgpd/bgpd.c:890
    #2 0x7f48c9bde039 in hash_release lib/hash.c:209
    #3 0x55cfefe3373f in bgp_peer_conf_if_to_su_update bgpd/bgpd.c:1541
    #4 0x55cfefd0be7a in bgp_stop bgpd/bgp_fsm.c:1631
    #5 0x55cfefe4028f in peer_delete bgpd/bgpd.c:2362
    #6 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #7 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #8 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #9 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #10 0x7f48c9c87402 in vty_command lib/vty.c:526
    #11 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #12 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #13 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #14 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #15 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #16 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #17 0x55cfefc787f9 in _start (/usr/lib/frr/bgpd+0xe27f9)

0x62a0000d8440 is located 576 bytes inside of 23376-byte region [0x62a0000d8200,0x62a0000ddd50)
freed by thread T0 here:
    #0 0x7f48c9eb9fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
    #1 0x55cfefe3fe42 in peer_free bgpd/bgpd.c:1113
    #2 0x55cfefe3fe42 in peer_unlock_with_caller bgpd/bgpd.c:1144
    #3 0x55cfefe4092e in peer_delete bgpd/bgpd.c:2457
    #4 0x55cfefdd5e97 in no_neighbor_interface_config bgpd/bgp_vty.c:4267
    #5 0x7f48c9b9d160 in cmd_execute_command_real lib/command.c:949
    #6 0x7f48c9ba1112 in cmd_execute_command lib/command.c:1009
    #7 0x7f48c9ba1573 in cmd_execute lib/command.c:1162
    #8 0x7f48c9c87402 in vty_command lib/vty.c:526
    #9 0x7f48c9c87832 in vty_execute lib/vty.c:1291
    #10 0x7f48c9c8e741 in vtysh_read lib/vty.c:2130
    #11 0x7f48c9c7a66d in thread_call lib/thread.c:1585
    #12 0x7f48c9bf64e7 in frr_run lib/libfrr.c:1123
    #13 0x55cfefc75a15 in main bgpd/bgp_main.c:540
    #14 0x7f48c96b009a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants