Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow abstraction of ECMP and Routes Installable #2

Closed
donaldsharp opened this issue Dec 16, 2016 · 5 comments
Closed

Allow abstraction of ECMP and Routes Installable #2

donaldsharp opened this issue Dec 16, 2016 · 5 comments

Comments

@donaldsharp
Copy link
Member

Currently we have 2 ways of specifying ECMP

  1. Via compilation of FRR
  2. Via runtime specification

Let's add a 3rd ability to query the kernel( or relevant api ) for any underlying hardware abstraction

Then after we have this input. Choose the min of the 3 to use at run time

Also add the same thing for # of routes. Some hardware has a limitation on the # of routes that can be installed. Allow the code to smartly understand that.

@donaldsharp donaldsharp mentioned this issue Dec 16, 2016
@pguibert6WIND
Copy link
Member

Hello,

unless i misunderstood, I do not agree with proposed solution.
I understand that if enable-multipath=64 is set by compilation, and bgpd command maximum-path 32 is chosen, and if the kernel support for multipath is 1, then that means that BGP global RIB does not support ECMP routes.

If this is the correct understanding, then I disagree. Unless the 3rd ability impacts only zebra. Meaning that BGP will still be able to handle ECMP routes in its global RIB.
Reason is that sometimes the Frr may be used as purely controlplane feature, not related to dataplane ( since openvswitch will be configured separately to configure the kernel stuff instead through a SDNc).

@riw777
Copy link
Member

riw777 commented Mar 1, 2017

I think this might be in support of various chip set's ability to modify the next hop rather than just replace the entire route (?) -- what you'd need to know in this case is what the max number of next hops (ie the size of the ecmp group) is that the hardware chipset can support, so zebra doesn't overrun that number, and blow up the modification of the ecmp group at the hardware level.

How would zebra get this information, though? Is there an existing api in that direction (from sai, open nsl, or just flat out from the chip api)?

I could be all wrong in reading the bug by Donald, though -- so ignore me if I'm blathering nonsense. :-)

@donaldsharp
Copy link
Member Author

So we have several issues here:

Hardware ECMP support
Kernel ECMP support
Zebra ECMP support

Currently there is no methodology to have any of these 3 layers know about what is supported under them.

Additionally one can use the zebra FPM manager to tell the hardware about the route, and bypass kernel installation altogether

What I am proposing here is the creation of a API where zebra can ask for the underlying ECMP supported and dissallow the exact situation you are concerned about above at the level of zebra.

I was going to write this code so that it was abstract enough that zebra just asks for total ECMP and whomever is programming for a particular piece of hardware can have zebra do the right thing. I could then write handlers for my hardware types and have them return the right thing.

Long term I would suggest that the Linux kernel needs to get an API where it could report on the underlying hardwares ECMP and then we could just write that bit of code to get it via a handler.

donaldsharp pushed a commit that referenced this issue May 30, 2017
redhat: use %initsystem check that works when chrooted
qlyoung referenced this issue in qlyoung/frr Jul 20, 2017
…eset

* rip_interface.c: Default for split_horizon_default differed between
  rip_interface_new and rip_interface_reset, causing at least some issues
  after interface events. See patchwork FRRouting#604. Fix, and consolidate code.

  (rip_interface_{reset,clean}) rename these to 'interface', as that's more
  appropriate.  Spin the ri specific bodies of these functions out to
  rip_interface_{reset,clean} helpers.  Factor out the overlaps, so
  rip_interface_reset uses rip_interface_clean.

  (rip_interface_new) just use rip_interface_reset.

* ripd.h: Update for (rip_interface_{reset,clean})

Reported by xufeng zhang, with a suggested fix on which this commit expands.
See patchwork FRRouting#604.  This commit addresses only the split-horizon
discrepency, issue #2.  The other issue they reported, #1, is not addressed,
though suggested fix seems inappropriate.

Cc: xufeng.zhang@windriver.com
ppmathis added a commit to ppmathis/frr that referenced this issue Jun 13, 2018
ppmathis added a commit to ppmathis/frr that referenced this issue Jun 14, 2018
ppmathis added a commit to ppmathis/frr that referenced this issue Jun 14, 2018
cscarpitta added a commit to cscarpitta/frr that referenced this issue May 8, 2024
Fix the following memory leaks found by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    FRRouting#3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    FRRouting#4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    FRRouting#3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#8 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    FRRouting#3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    FRRouting#4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    FRRouting#5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    FRRouting#6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    FRRouting#7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    FRRouting#8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
cscarpitta added a commit to cscarpitta/frr that referenced this issue May 8, 2024
Fix a couple of memory leaks spotted by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    FRRouting#3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    FRRouting#4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    FRRouting#3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#8 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    FRRouting#3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    FRRouting#4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    FRRouting#5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    FRRouting#6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    FRRouting#7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    FRRouting#8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
cscarpitta added a commit to cscarpitta/frr that referenced this issue May 9, 2024
Fix a couple of memory leaks spotted by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    FRRouting#3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    FRRouting#4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    FRRouting#3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#8 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    FRRouting#3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    FRRouting#4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    FRRouting#5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    FRRouting#6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    FRRouting#7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    FRRouting#8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
mergify bot pushed a commit that referenced this issue May 9, 2024
Fix a couple of memory leaks spotted by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    #3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    #4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    #5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    #6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    #7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    #8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    #9 0xfeb98af3d684 in event_call lib/event.c:2011
    #10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    #3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    #4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    #5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    #6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    #7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    #8 0xfeb98af3d684 in event_call lib/event.c:2011
    #9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    #3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    #4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    #5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    #6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    #7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    #8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    #9 0xfeb98af3d684 in event_call lib/event.c:2011
    #10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
(cherry picked from commit 65e0111)
mergify bot pushed a commit that referenced this issue May 9, 2024
Fix a couple of memory leaks spotted by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    #3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    #4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    #5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    #6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    #7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    #8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    #9 0xfeb98af3d684 in event_call lib/event.c:2011
    #10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    #3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    #4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    #5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    #6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    #7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    #8 0xfeb98af3d684 in event_call lib/event.c:2011
    #9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    #2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    #3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    #4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    #5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    #6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    #7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    #8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    #9 0xfeb98af3d684 in event_call lib/event.c:2011
    #10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    #11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    #12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    #14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
(cherry picked from commit 65e0111)
EasyNetDev pushed a commit to EasyNetDev/frr that referenced this issue May 13, 2024
Fix a couple of memory leaks spotted by Address Sanitizer:

```

=================================================================
==970960==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 592 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    FRRouting#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
    FRRouting#3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
    FRRouting#4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    FRRouting#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
    FRRouting#3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
    FRRouting#4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
    FRRouting#5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
    FRRouting#6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
    FRRouting#7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
    FRRouting#8 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    FRRouting#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
    FRRouting#2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
    FRRouting#3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
    FRRouting#4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
    FRRouting#5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
    FRRouting#6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
    FRRouting#7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
    FRRouting#8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
    FRRouting#9 0xfeb98af3d684 in event_call lib/event.c:2011
    FRRouting#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
    FRRouting#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
    FRRouting#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    FRRouting#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
    FRRouting#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)

SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).

```

Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
ton31337 pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
mergify bot pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de5)
mergify bot pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de5)
mergify bot pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de5)
mergify bot pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de5)
mergify bot pushed a commit that referenced this issue May 23, 2024
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
>     #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
>     #1 0x7f84b8204f71 in hash_get lib/hash.c:142
>     #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
>     #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
>     #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
>     #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #11 0x7f84b835c72d in event_call lib/event.c:2011
>     #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #13 0x563828c21918 in main isisd/isis_main.c:346
>     #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>     #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
>     #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7f84b8263bae in qfree lib/memory.c:130
>     #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
>     #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
>     #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #9 0x7f84b835c72d in event_call lib/event.c:2011
>     #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #11 0x563828c21918 in main isisd/isis_main.c:346
>     #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
>     #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
>     #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
>     #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
>     #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
>     #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
>     #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
>     #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
>     #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
>     #10 0x7f84b835c72d in event_call lib/event.c:2011
>     #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
>     #12 0x563828c21918 in main isisd/isis_main.c:346
>     #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
>   0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
>   0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>   0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2334217==ABORTING

Fixes: 2f7cc7b ("isisd: detect Prefix-SID collisions and handle them appropriately")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de5)
mergify bot pushed a commit that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    #2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7f8026a88d3f in hash_get lib/hash.c:147
    #4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    #6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    #7 0x7f8026b62e0e in event_call lib/event.c:1969
    #8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    #9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    #10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    #2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    #4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    #6 0x7fb6e373ee0e in event_call lib/event.c:1969
    #7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    #8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    #9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
(cherry picked from commit d163f89)

# Conflicts:
#	nhrpd/nhrp_main.c
mergify bot pushed a commit that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    #2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7f8026a88d3f in hash_get lib/hash.c:147
    #4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    #6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    #7 0x7f8026b62e0e in event_call lib/event.c:1969
    #8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    #9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    #10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    #2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    #4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    #6 0x7fb6e373ee0e in event_call lib/event.c:1969
    #7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    #8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    #9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
(cherry picked from commit d163f89)
mergify bot pushed a commit that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    #2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7f8026a88d3f in hash_get lib/hash.c:147
    #4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    #6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    #7 0x7f8026b62e0e in event_call lib/event.c:1969
    #8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    #9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    #10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    #2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    #4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    #6 0x7fb6e373ee0e in event_call lib/event.c:1969
    #7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    #8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    #9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
(cherry picked from commit d163f89)

# Conflicts:
#	nhrpd/nhrp_cache.c
#	nhrpd/nhrp_peer.c
mergify bot pushed a commit that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    #2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7f8026a88d3f in hash_get lib/hash.c:147
    #4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    #6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    #7 0x7f8026b62e0e in event_call lib/event.c:1969
    #8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    #9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    #10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    #2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    #4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    #6 0x7fb6e373ee0e in event_call lib/event.c:1969
    #7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    #8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    #9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
(cherry picked from commit d163f89)

# Conflicts:
#	nhrpd/nhrp_cache.c
#	nhrpd/nhrp_peer.c
donaldsharp pushed a commit that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    #2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7f8026a88d3f in hash_get lib/hash.c:147
    #4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    #6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    #7 0x7f8026b62e0e in event_call lib/event.c:1969
    #8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    #9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    #10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    #2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    #3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    #4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    #5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    #6 0x7fb6e373ee0e in event_call lib/event.c:1969
    #7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    #8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    #9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Jafaral pushed a commit to Jafaral/frr that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7f8026a88d3f in hash_get lib/hash.c:147
    FRRouting#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    FRRouting#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    FRRouting#7 0x7f8026b62e0e in event_call lib/event.c:1969
    FRRouting#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    FRRouting#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    FRRouting#10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    FRRouting#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    FRRouting#6 0x7fb6e373ee0e in event_call lib/event.c:1969
    FRRouting#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    FRRouting#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    FRRouting#9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Jafaral pushed a commit to Jafaral/frr that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7f8026a88d3f in hash_get lib/hash.c:147
    FRRouting#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    FRRouting#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    FRRouting#7 0x7f8026b62e0e in event_call lib/event.c:1969
    FRRouting#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    FRRouting#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    FRRouting#10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    FRRouting#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    FRRouting#6 0x7fb6e373ee0e in event_call lib/event.c:1969
    FRRouting#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    FRRouting#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    FRRouting#9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Jafaral pushed a commit to Jafaral/frr that referenced this issue Jun 5, 2024
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
  thereby causing a memory leak.

- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.

- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265

=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7f8026a88d3f in hash_get lib/hash.c:147
    FRRouting#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
    FRRouting#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
    FRRouting#7 0x7f8026b62e0e in event_call lib/event.c:1969
    FRRouting#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
    FRRouting#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
    FRRouting#10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400

=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    FRRouting#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
    FRRouting#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
    FRRouting#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
    FRRouting#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
    FRRouting#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
    FRRouting#6 0x7fb6e373ee0e in event_call lib/event.c:1969
    FRRouting#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
    FRRouting#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
    FRRouting#9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
riw777 pushed a commit that referenced this issue Jun 24, 2024
Fix a crash when doing "show isis database detail json" in
isis_srv6_topo1 topotest.

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fad89524e2c in core_handler (signo=6, siginfo=0x7ffe86a4b8b0, context=0x7ffe86a4b780) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> #4  0x00007fad8904e537 in __GI_abort () at abort.c:79
> #5  0x00007fad8904e40f in __assert_fail_base (fmt=0x7fad891c5688 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7fad8a3e70e8 "json_object_get_type(jso) == json_type_object",
>     file=0x7fad8a3e7064 "./json_object.c", line=590, function=<optimized out>) at assert.c:92
> #6  0x00007fad8905d662 in __GI___assert_fail (assertion=0x7fad8a3e70e8 "json_object_get_type(jso) == json_type_object", file=0x7fad8a3e7064 "./json_object.c", line=590,
>     function=0x7fad8a3e7440 "json_object_object_add_ex") at assert.c:101
> #7  0x00007fad8a3dfe93 in json_object_object_add_ex () from /lib/x86_64-linux-gnu/libjson-c.so.5
> #8  0x000055708e3f8f7f in format_subsubtlv_srv6_sid_structure (sid_struct=0x602000172b70, buf=0x0, json=0x6040000a21d0, indent=6) at isisd/isis_tlvs.c:2880
> #9  0x000055708e3f9acb in isis_format_subsubtlvs (subsubtlvs=0x602000172b50, buf=0x0, json=0x6040000a21d0, indent=6) at isisd/isis_tlvs.c:3022
> #10 0x000055708e3eefb0 in format_item_ext_subtlvs (exts=0x614000047440, buf=0x0, json=0x6040000a2190, indent=2, mtid=2) at isisd/isis_tlvs.c:1313
> #11 0x000055708e3fd599 in format_item_extended_reach (mtid=2, i=0x60300015aed0, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:3763
> #12 0x000055708e40d46a in format_item (mtid=2, context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, i=0x60300015aed0, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:6789
> #13 0x000055708e40d4fc in format_items_ (mtid=2, context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, items=0x60600021d160, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:6804
> #14 0x000055708e40edbc in format_mt_items (context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, m=0x6180000845d8, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:7147
> #15 0x000055708e4111e9 in format_tlvs (tlvs=0x618000084480, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:7572
> #16 0x000055708e4114ce in isis_format_tlvs (tlvs=0x618000084480, json=0x6040000a1bd0) at isisd/isis_tlvs.c:7613
> #17 0x000055708e36f167 in lsp_print_detail (lsp=0x612000058b40, vty=0x0, json=0x6040000a1bd0, dynhost=1 '\001', isis=0x60d00001f800) at isisd/isis_lsp.c:785
> #18 0x000055708e36f31f in lsp_print_all (vty=0x0, json=0x6040000a0490, head=0x61f000005488, detail=1 '\001', dynhost=1 '\001', isis=0x60d00001f800) at isisd/isis_lsp.c:820
> #19 0x000055708e4379fc in show_isis_database_lspdb_json (json=0x6040000a0450, area=0x61f000005480, level=0, lspdb=0x61f000005488, sysid_str=0x0, ui_level=1) at isisd/isisd.c:2683
> #20 0x000055708e437ef9 in show_isis_database_json (json=0x6040000a0310, sysid_str=0x0, ui_level=1, isis=0x60d00001f800) at isisd/isisd.c:2754
> #21 0x000055708e438357 in show_isis_database_common (vty=0x62e000060400, json=0x6040000a0310, sysid_str=0x0, ui_level=1, isis=0x60d00001f800) at isisd/isisd.c:2788
> #22 0x000055708e438591 in show_isis_database (vty=0x62e000060400, json=0x6040000a0310, sysid_str=0x0, ui_level=1, vrf_name=0x7fad89806300 <vrf_default_name> "default", all_vrf=false)
>     at isisd/isisd.c:2825
> #23 0x000055708e43891d in show_database (self=0x55708e5519c0 <show_database_cmd>, vty=0x62e000060400, argc=5, argv=0x6040000a02d0) at isisd/isisd.c:2855
> #24 0x00007fad893a9767 in cmd_execute_command_real (vline=0x60300015f220, vty=0x62e000060400, cmd=0x0, up_level=0) at lib/command.c:1002
> #25 0x00007fad893a9adc in cmd_execute_command (vline=0x60300015f220, vty=0x62e000060400, cmd=0x0, vtysh=0) at lib/command.c:1061
> #26 0x00007fad893aa728 in cmd_execute (vty=0x62e000060400, cmd=0x621000025900 "show isis database detail json ", matched=0x0, vtysh=0) at lib/command.c:1227

Note that prior to 2e670cd, there was no crash but only the last
"srv6-sid-structure" was displayed. A "srv6-sid-structure" should be
displayed for each "sid". This commit also fix this.

Was:

> "srv6-lan-endx-sid": [
>   {
>     "sid": "fc00:0:1:1::",
>     "weight": 0,
>     "algorithm": "SPF",
>     "neighbor-id": "0000.0000.0002"
>   },
>   {
>     "sid": "fc00:0:1:2::",
>     "weight": 0,
>     "algorithm": "SPF",
>     "neighbor-id": "0000.0000.0003"
>   }
> ],
> "srv6-sid-structure": {
>   "loc-block-len": 32,
>   "loc-node-len": 16,
>   "func-len": 16,
>   "arg-len": 0
> },

Now (srv6-sid-structure are identical but they are not always):

> "srv6-lan-endx-sid": [
>   {
>     "sid": "fc00:0:1:1::",
>     "algorithm": "SPF",
>     "neighbor-id": "0000.0000.0002",
>     "srv6-sid-structure": {
>       "loc-block-len": 32,
>       "loc-node-len": 16,
>       "func-len": 8,
>       "arg-len": 0
>     },
>   },
>   {
>     "sid": "fc00:0:1:2::",
>     "algorithm": "SPF",
>     "neighbor-id": "0000.0000.0003",
>     "srv6-sid-structure": {
>       "loc-block-len": 32,
>       "loc-node-len": 16,
>       "func-len": 16,
>       "arg-len": 0
>     },
>   }
> ],

Fixes: 2e670cd ("isisd: fix display of srv6 subsubtlvs")
Fixes: 648a158 ("isisd: Add SRv6 End.X SID to Sub-TLV format func")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
riw777 pushed a commit that referenced this issue Jul 2, 2024
…links

When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in area->adjacency_list may be inconsistent with the neighbors in lsp->tlvs->oldstyle_reach/extended_reach. For example, the area->adjacency_list may lack certain neighbors even though they are present in the LSP. When computing SPF, the call to isis_spf_build_adj_list() generates the spftree->sadj_list, which reflects the real neighbors in the area->adjacency_list. However, in the case of LAN links, spftree->sadj_list may include additional pseudo neighbors.
The pre-loading of tents through the call to isis_spf_preload_tent involves two steps:
1. isis_spf_process_lsp() is called to generate real neighbor vertices based on the root LSP and pseudo LSP.
2. isis_spf_add_local() is called to add corresponding next hops to the vertex->Adj_N list for the real neighbor vertices.
In the case of LAN links, the absence of corresponding real neighbors in the spftree->sadj_list prevents the execution of the second step. Consequently, the vertex->Adj_N list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access when isis_lfa_compute() is called to calculate LFA. 
As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue.
The backtrace is as follows:
(gdb) bt
#0  0x00007fd065277fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007fd065398972 in core_handler (signo=11, siginfo=0x7ffc5c0636b0, context=0x7ffc5c063580) at ../lib/sigevent.c:261
#2  <signal handler called>
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
#4  0x00005564d82f8d78 in isis_spf_run_lfa (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_lfa.c:2344
#5  0x00005564d8315964 in isis_run_spf_with_protection (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_spf.c:1827
#6  0x00005564d8315c15 in isis_run_spf_cb (thread=0x7ffc5c064590) at ../isisd/isis_spf.c:1889
#7  0x00007fd0653b1f04 in thread_call (thread=0x7ffc5c064590) at ../lib/thread.c:1990
#8  0x00007fd06534a97b in frr_run (master=0x5564d88103c0) at ../lib/libfrr.c:1198
#9  0x00005564d82e7d5d in main (argc=5, argv=0x7ffc5c0647b8, envp=0x7ffc5c0647e8) at ../isisd/isis_main.c:273
(gdb) f 3
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
2134    ../isisd/isis_lfa.c: No such file or directory.
(gdb) p vadj_primary
$1 = (struct isis_vertex_adj *) 0x0
(gdb) p vertex->Adj_N->head
$2 = (struct listnode *) 0x0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->data
$8 = (struct isis_vertex *) 0x5564d8b5b240
(gdb) p $8->type
$9 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $8->N.id
$10 = "\000\000\000\000\000\002"
(gdb) p $8->Adj_N->count
$11 = 0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->next->data
$12 = (struct isis_vertex *) 0x5564d8b73dd0
(gdb) p $12->type
$13 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $12->N.id
$14 = "\000\000\000\000\000\003"
(gdb) p $12->Adj_N->count
$15 = 0
(gdb) p area->adjacency_list->count
$16 = 0
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes isis_vertex_adj_del() to delete the next hop of a vertex. Upon reviewing all the instances where the vertex->Adj_N list is used, I found that only isis_lfa_compute() lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, the vertex->parents list for IP vertices is guaranteed not to be empty.
Test scenario:
Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue.

Signed-off-by: zhou-run <zhou.run@h3c.com>
mergify bot pushed a commit that referenced this issue Jul 2, 2024
…links

When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in area->adjacency_list may be inconsistent with the neighbors in lsp->tlvs->oldstyle_reach/extended_reach. For example, the area->adjacency_list may lack certain neighbors even though they are present in the LSP. When computing SPF, the call to isis_spf_build_adj_list() generates the spftree->sadj_list, which reflects the real neighbors in the area->adjacency_list. However, in the case of LAN links, spftree->sadj_list may include additional pseudo neighbors.
The pre-loading of tents through the call to isis_spf_preload_tent involves two steps:
1. isis_spf_process_lsp() is called to generate real neighbor vertices based on the root LSP and pseudo LSP.
2. isis_spf_add_local() is called to add corresponding next hops to the vertex->Adj_N list for the real neighbor vertices.
In the case of LAN links, the absence of corresponding real neighbors in the spftree->sadj_list prevents the execution of the second step. Consequently, the vertex->Adj_N list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access when isis_lfa_compute() is called to calculate LFA.
As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue.
The backtrace is as follows:
(gdb) bt
#0  0x00007fd065277fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007fd065398972 in core_handler (signo=11, siginfo=0x7ffc5c0636b0, context=0x7ffc5c063580) at ../lib/sigevent.c:261
#2  <signal handler called>
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
#4  0x00005564d82f8d78 in isis_spf_run_lfa (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_lfa.c:2344
#5  0x00005564d8315964 in isis_run_spf_with_protection (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_spf.c:1827
#6  0x00005564d8315c15 in isis_run_spf_cb (thread=0x7ffc5c064590) at ../isisd/isis_spf.c:1889
#7  0x00007fd0653b1f04 in thread_call (thread=0x7ffc5c064590) at ../lib/thread.c:1990
#8  0x00007fd06534a97b in frr_run (master=0x5564d88103c0) at ../lib/libfrr.c:1198
#9  0x00005564d82e7d5d in main (argc=5, argv=0x7ffc5c0647b8, envp=0x7ffc5c0647e8) at ../isisd/isis_main.c:273
(gdb) f 3
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
2134    ../isisd/isis_lfa.c: No such file or directory.
(gdb) p vadj_primary
$1 = (struct isis_vertex_adj *) 0x0
(gdb) p vertex->Adj_N->head
$2 = (struct listnode *) 0x0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->data
$8 = (struct isis_vertex *) 0x5564d8b5b240
(gdb) p $8->type
$9 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $8->N.id
$10 = "\000\000\000\000\000\002"
(gdb) p $8->Adj_N->count
$11 = 0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->next->data
$12 = (struct isis_vertex *) 0x5564d8b73dd0
(gdb) p $12->type
$13 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $12->N.id
$14 = "\000\000\000\000\000\003"
(gdb) p $12->Adj_N->count
$15 = 0
(gdb) p area->adjacency_list->count
$16 = 0
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes isis_vertex_adj_del() to delete the next hop of a vertex. Upon reviewing all the instances where the vertex->Adj_N list is used, I found that only isis_lfa_compute() lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, the vertex->parents list for IP vertices is guaranteed not to be empty.
Test scenario:
Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue.

Signed-off-by: zhou-run <zhou.run@h3c.com>
(cherry picked from commit a970bb5)
mergify bot pushed a commit that referenced this issue Jul 2, 2024
…links

When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in area->adjacency_list may be inconsistent with the neighbors in lsp->tlvs->oldstyle_reach/extended_reach. For example, the area->adjacency_list may lack certain neighbors even though they are present in the LSP. When computing SPF, the call to isis_spf_build_adj_list() generates the spftree->sadj_list, which reflects the real neighbors in the area->adjacency_list. However, in the case of LAN links, spftree->sadj_list may include additional pseudo neighbors.
The pre-loading of tents through the call to isis_spf_preload_tent involves two steps:
1. isis_spf_process_lsp() is called to generate real neighbor vertices based on the root LSP and pseudo LSP.
2. isis_spf_add_local() is called to add corresponding next hops to the vertex->Adj_N list for the real neighbor vertices.
In the case of LAN links, the absence of corresponding real neighbors in the spftree->sadj_list prevents the execution of the second step. Consequently, the vertex->Adj_N list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access when isis_lfa_compute() is called to calculate LFA.
As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue.
The backtrace is as follows:
(gdb) bt
#0  0x00007fd065277fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007fd065398972 in core_handler (signo=11, siginfo=0x7ffc5c0636b0, context=0x7ffc5c063580) at ../lib/sigevent.c:261
#2  <signal handler called>
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
#4  0x00005564d82f8d78 in isis_spf_run_lfa (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_lfa.c:2344
#5  0x00005564d8315964 in isis_run_spf_with_protection (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_spf.c:1827
#6  0x00005564d8315c15 in isis_run_spf_cb (thread=0x7ffc5c064590) at ../isisd/isis_spf.c:1889
#7  0x00007fd0653b1f04 in thread_call (thread=0x7ffc5c064590) at ../lib/thread.c:1990
#8  0x00007fd06534a97b in frr_run (master=0x5564d88103c0) at ../lib/libfrr.c:1198
#9  0x00005564d82e7d5d in main (argc=5, argv=0x7ffc5c0647b8, envp=0x7ffc5c0647e8) at ../isisd/isis_main.c:273
(gdb) f 3
#3  0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
2134    ../isisd/isis_lfa.c: No such file or directory.
(gdb) p vadj_primary
$1 = (struct isis_vertex_adj *) 0x0
(gdb) p vertex->Adj_N->head
$2 = (struct listnode *) 0x0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->data
$8 = (struct isis_vertex *) 0x5564d8b5b240
(gdb) p $8->type
$9 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $8->N.id
$10 = "\000\000\000\000\000\002"
(gdb) p $8->Adj_N->count
$11 = 0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->next->data
$12 = (struct isis_vertex *) 0x5564d8b73dd0
(gdb) p $12->type
$13 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $12->N.id
$14 = "\000\000\000\000\000\003"
(gdb) p $12->Adj_N->count
$15 = 0
(gdb) p area->adjacency_list->count
$16 = 0
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes isis_vertex_adj_del() to delete the next hop of a vertex. Upon reviewing all the instances where the vertex->Adj_N list is used, I found that only isis_lfa_compute() lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, the vertex->parents list for IP vertices is guaranteed not to be empty.
Test scenario:
Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue.

Signed-off-by: zhou-run <zhou.run@h3c.com>
(cherry picked from commit a970bb5)
riw777 pushed a commit that referenced this issue Jul 16, 2024
… the fragmented LSP

1. When the root IS regenerates an LSP, it calls lsp_build() -> lsp_clear_data() to free the TLV memory of the first fragment and all other fragments. If the number of fragments in the regenerated LSP decreases or if no fragmentation is needed, the extra LSP fragments are not immediately deleted. Instead, lsp_seqno_update() -> lsp_purge() is called to set the remaining time to zero and start aging, while also notifying other IS nodes to age these fragments. lsp_purge() usually does not reset lsp->hdr.seqno to zero because the LSP might recover during the aging process.
2. When other IS nodes receive an LSP, they always call process_lsp() -> isis_unpack_tlvs() to allocate TLV memory for the LSP. This does not differentiate whether the received LSP has a remaining lifetime of zero. Therefore, it is rare for an LSP of a non-root IS to have empty TLVs. Of course, if an LSP with a remaining time of zero and already corrupted is received, lsp_update() -> lsp_purge() will be called to free the TLV memory of the LSP, but this scenario is rare.
3. In LFA calculations, neighbors of the root IS are traversed, and each neighbor is taken as a new root to compute the neighbor SPT. During this process, the old root IS will serve as a neighbor of the new root IS, triggering a call to isis_spf_process_lsp() to parse the LSP of the old root IS and obtain its IP vertices and neighboring IS vertices. However, isis_spf_process_lsp() only checks whether the TLVs in the first fragment of the LSP exist, and does not check the TLVs in the fragmented LSP. If the TLV memory of the fragmented LSP of the old root IS has been freed, it can lead to a null pointer access, causing the current crash.

Additionally, for the base SPT, there are only two places where the LSP of the root IS is parsed:
1. When obtaining the UP neighbors of the root IS via spf_adj_list_parse_lsp().
2. When preloading the IP vertices of the root IS via isis_lsp_iterate_ip_reach().
Both of these checks ensure that frag->tlvs is not null, and they do not subsequently call isis_spf_process_lsp() to parse the root IS's LSP. It is very rare for non-root IS LSPs to have empty TLVs unless they are corrupted LSPs awaiting deletion. If it happens, a crash will occur.

The backtrace is as follows:
(gdb) bt
#0  0x00007f3097281fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007f30973a2972 in core_handler (signo=11, siginfo=0x7ffce66c2870, context=0x7ffce66c2740) at ../lib/sigevent.c:261
#2  <signal handler called>
#3  0x000055dfa805512b in isis_spf_process_lsp (spftree=0x55dfa950eee0, lsp=0x55dfa94cb590, cost=10, depth=1, root_sysid=0x55dfa950ef6c "", parent=0x55dfa952fca0)
    at ../isisd/isis_spf.c:898
#4  0x000055dfa805743b in isis_spf_loop (spftree=0x55dfa950eee0, root_sysid=0x55dfa950ef6c "") at ../isisd/isis_spf.c:1688
#5  0x000055dfa805784f in isis_run_spf (spftree=0x55dfa950eee0) at ../isisd/isis_spf.c:1808
#6  0x000055dfa8037ff5 in isis_spf_run_neighbors (spftree=0x55dfa9474440) at ../isisd/isis_lfa.c:1259
#7  0x000055dfa803ac17 in isis_spf_run_lfa (area=0x55dfa9477510, spftree=0x55dfa9474440) at ../isisd/isis_lfa.c:2300
#8  0x000055dfa8057964 in isis_run_spf_with_protection (area=0x55dfa9477510, spftree=0x55dfa9474440) at ../isisd/isis_spf.c:1827
#9  0x000055dfa8057c15 in isis_run_spf_cb (thread=0x7ffce66c38e0) at ../isisd/isis_spf.c:1889
#10 0x00007f30973bbf04 in thread_call (thread=0x7ffce66c38e0) at ../lib/thread.c:1990
#11 0x00007f309735497b in frr_run (master=0x55dfa91733c0) at ../lib/libfrr.c:1198
#12 0x000055dfa8029d5d in main (argc=5, argv=0x7ffce66c3b08, envp=0x7ffce66c3b38) at ../isisd/isis_main.c:273
(gdb) f 3
#3  0x000055dfa805512b in isis_spf_process_lsp (spftree=0x55dfa950eee0, lsp=0x55dfa94cb590, cost=10, depth=1, root_sysid=0x55dfa950ef6c "", parent=0x55dfa952fca0)
    at ../isisd/isis_spf.c:898
898     ../isisd/isis_spf.c: No such file or directory.
(gdb) p te_neighs
$1 = (struct isis_item_list *) 0x120
(gdb) p lsp->tlvs
$2 = (struct isis_tlvs *) 0x0
(gdb) p lsp->hdr
$3 = {pdu_len = 27, rem_lifetime = 0, lsp_id = "\000\000\000\000\000\001\000\001", seqno = 4, checksum = 59918, lsp_bits = 1 '\001'}

The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.

I have reviewed the process for calculating the SPT based on the LSP, and isis_spf_process_lsp() is the only function that does not check whether the TLVs in the fragments are empty. Therefore, I believe that modifying this function alone should be sufficient. If the TLVs of the current fragment are already empty, we do not need to continue processing subsequent fragments. This is consistent with the behavior where we do not process fragments if the TLVs of the first fragment are empty.
Of course, one could argue that lsp_purge() should still retain the TLV memory, freeing it and then reallocating it if needed. However, this is a debatable point because in some scenarios, it is permissible for the LSP to have empty TLVs. For example, after receiving an SNP (Sequence Number PDU) message, an empty LSP (with lsp->hdr.seqno = 0) might be created by calling lsp_new. If the corresponding LSP message is discarded due to domain or area authentication failure, the TLV memory wouldn't be allocated.

Test scenario:
In an LFA network, importing a sufficient number of static routes to cause LSP fragmentation, and then rolling back the imported static routes so that the LSP is no longer fragmented, can easily result in this issue.


Signed-off-by: zhou-run <zhou.run@h3c.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants