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.

Sign up for GitHub

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

Jump to bottom

zebra: break FreeBSD compile #12

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

zebra: break FreeBSD compile #12

wants to merge 5 commits into from

Conversation

mwinter-osr
Copy link
Member

Github Checks Test - PR should break FreeBSD build

@mwinter-osr
Copy link
Member Author

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

9 similar comments
@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@mwinter-osr
zebra: Break all compile
dda02ed 
Signed-off-by: Martin Winter <[email protected]>
@mwinter-osr
Revert "zebra: Break all compile"
3823985 
This reverts commit dda02ed.

Signed-off-by: Martin Winter <[email protected]>
@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

3 similar comments
@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

CI:ReRun Try compiling again

@RodrigoMNardi
Copy link
Contributor

ci:rerun

RodrigoMNardi pushed a commit that referenced this pull request Jan 15, 2024
@louis-6wind
bgpd: fix crash when re-adding a rpki server
51a97a0 
Fix a crash when re-adding a rpki server:

> r2# sh run bgpd
> [...]
> rpki
>  rpki retry_interval 5
>  rpki cache 192.0.2.1 15432 preference 1
> exit
> [...]
> r2# conf t
> r2(config)# rpki
> r2(config-rpki)# no rpki cache 192.0.2.1 15432 preference 1
> r2(config-rpki)# do show rpki cache-connection
> Cannot find a connected group.
> r2(config-rpki)# rpki cache 192.0.2.1 15432 preference 1
> r2(config-rpki)# do show rpki cache-connection
> vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error!

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f3fd2d16e57 in core_handler (signo=11, siginfo=0x7ffffd5931b0, context=0x7ffffd593080) at lib/sigevent.c:246
> #2  <signal handler called>
> #3  0x00007f3fd26926b4 in tommy_list_head (list=0x2e322e302e323931) at /home/lscalber/git/rtrlib/./third-party/tommyds/tommylist.h:125
> #4  0x00007f3fd2693812 in rtr_mgr_get_first_group (config=0x55fbf31d7f00) at /home/lscalber/git/rtrlib/rtrlib/rtr_mgr.c:409
> #5  0x00007f3fd2ebef59 in get_connected_group () at bgpd/bgp_rpki.c:718
> #6  0x00007f3fd2ec0b39 in show_rpki_cache_connection_magic (self=0x7f3fd2ec69c0 <show_rpki_cache_connection_cmd>, vty=0x55fbf31f9ef0, argc=3, argv=0x55fbf31f99d0, uj=0x0)
> #   at bgpd/bgp_rpki.c:1575
> #7  0x00007f3fd2ebd4da in show_rpki_cache_connection (self=0x7f3fd2ec69c0 <show_rpki_cache_connection_cmd>, vty=0x55fbf31f9ef0, argc=3, argv=0x55fbf31f99d0) at ./bgpd/bgp_rpki_clippy.c:648
> #8  0x00007f3fd2c8a142 in cmd_execute_command_real (vline=0x55fbf31f9990, vty=0x55fbf31f9ef0, cmd=0x0, up_level=0) at lib/command.c:978
> #9  0x00007f3fd2c8a25c in cmd_execute_command (vline=0x55fbf31e5260, vty=0x55fbf31f9ef0, cmd=0x0, vtysh=0) at lib/command.c:1028
> #10 0x00007f3fd2c8a7f1 in cmd_execute (vty=0x55fbf31f9ef0, cmd=0x55fbf3200680 "do show rpki cache-connection ", matched=0x0, vtysh=0) at lib/command.c:1203
> #11 0x00007f3fd2d36548 in vty_command (vty=0x55fbf31f9ef0, buf=0x55fbf3200680 "do show rpki cache-connection ") at lib/vty.c:594
> #12 0x00007f3fd2d382e1 in vty_execute (vty=0x55fbf31f9ef0) at lib/vty.c:1357
> #13 0x00007f3fd2d3a519 in vtysh_read (thread=0x7ffffd5963c0) at lib/vty.c:2365
> #14 0x00007f3fd2d2faf6 in event_call (thread=0x7ffffd5963c0) at lib/event.c:1974
> #15 0x00007f3fd2cc238e in frr_run (master=0x55fbf2a0cd60) at lib/libfrr.c:1214
> #16 0x000055fbf073de40 in main (argc=9, argv=0x7ffffd596618) at bgpd/bgp_main.c:510

Signed-off-by: Louis Scalbert <[email protected]>
@RodrigoMNardi
Copy link
Contributor

CI:ReRun

2 similar comments
@RodrigoMNardi
Copy link
Contributor

CI:ReRun

@RodrigoMNardi
Copy link
Contributor

CI:ReRun

rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@louis-6wind
bgpd: fix logging from rpki_create_socket()
fdaf08b 
Fix the following crash when logging from rpki_create_socket():

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f6e21723798 in core_handler (signo=6, siginfo=0x7f6e1e502ef0, context=0x7f6e1e502dc0) at lib/sigevent.c:248
> #2  <signal handler called>
> #3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> #4  0x00007f6e2144e537 in __GI_abort () at abort.c:79
> #5  0x00007f6e2176348e in _zlog_assert_failed (xref=0x7f6e2180c920 <_xref.16>, extra=0x0) at lib/zlog.c:670
> #6  0x00007f6e216b1eda in rcu_read_lock () at lib/frrcu.c:294
> #7  0x00007f6e21762da8 in vzlog_notls (xref=0x0, prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed", ap=0x7f6e1e504248) at lib/zlog.c:425
> #8  0x00007f6e217632fb in vzlogx (xref=0x0, prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed", ap=0x7f6e1e504248) at lib/zlog.c:627
> #9  0x00007f6e217621f5 in zlog (prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed") at lib/zlog.h:73
> #10 0x00007f6e21763596 in _zlog_assert_failed (xref=0x7f6e2180c920 <_xref.16>, extra=0x0) at lib/zlog.c:687
> #11 0x00007f6e216b1eda in rcu_read_lock () at lib/frrcu.c:294
> #12 0x00007f6e21762da8 in vzlog_notls (xref=0x7f6e21a50040 <_xref.68>, prio=4, fmt=0x7f6e21a4999f "getaddrinfo: debug", ap=0x7f6e1e504878) at lib/zlog.c:425
> #13 0x00007f6e217632fb in vzlogx (xref=0x7f6e21a50040 <_xref.68>, prio=4, fmt=0x7f6e21a4999f "getaddrinfo: debug", ap=0x7f6e1e504878) at lib/zlog.c:627
> #14 0x00007f6e21a3f774 in zlog_ref (xref=0x7f6e21a50040 <_xref.68>, fmt=0x7f6e21a4999f "getaddrinfo: debug") at ./lib/zlog.h:84
> #15 0x00007f6e21a451b2 in rpki_create_socket (_cache=0x55729149cc30) at bgpd/bgp_rpki.c:1337
> #16 0x00007f6e2120e7b7 in tr_tcp_open (tr_socket=0x5572914d1520) at rtrlib/rtrlib/transport/tcp/tcp_transport.c:111
> #17 0x00007f6e2120e212 in tr_open (socket=0x5572914b5e00) at rtrlib/rtrlib/transport/transport.c:16
> #18 0x00007f6e2120faa2 in rtr_fsm_start (rtr_socket=0x557290e17180) at rtrlib/rtrlib/rtr/rtr.c:130
> #19 0x00007f6e218b7ea7 in start_thread (arg=<optimized out>) at pthread_create.c:477
> #20 0x00007f6e21527a2f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

rpki_create_socket() is a hook function called from the rtrlib library.
The issue arises because rtrlib initiates its own separate pthread in which
it runs the hook, which does not establish an FRR RCU context. Consequently,
this leads to failures in the logging mechanism that relies on RCU.

Initialize a new FRR pthread context from the rtrlib pthread with a
valid RCU context to allow logging from the rpki_create_socket() and
dependent functions.

Link: FRRouting/frr#15260
Fixes: a951752 ("bgpd: create cache server socket in vrf")
Signed-off-by: Louis Scalbert <[email protected]>
rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@fdumontet6WIND
bgpd: fix no bgp as-path access-list issue
100ef15 
router bgp 65001
 no bgp ebgp-requires-policy
 neighbor 192.168.1.2 remote-as external
 neighbor 192.168.1.2 timers 3 10
 address-family ipv4 unicast
  neighbor 192.168.1.2 route-map r2 in
 exit-address-family
!
ip prefix-list p1 seq 5 permit 172.16.255.31/32
!
route-map r2 permit 10
 match ip address prefix-list p1
 set as-path exclude 65003
route-map r2 permit 20
 set as-path exclude all
!

we make the following commands

bgp as-path access-list FIRST permit ^65
bgp as-path access-list SECOND permit 2
 route-map r2 permit 6
  set as-path exclude as-path-access-list SECOND

and then

no bgp as-path access-list SECOND permit 2
clear bgp *

we have the following crash in bgp

               Stack trace of thread 536083:
                #0  0x00007f87f8aacfe1 raise (libpthread.so.0 + 0x12fe1)
                #1  0x00007f87f8cf6870 core_handler (libfrr.so.0 +
		    0xf6870)
                #2  0x00007f87f8aad140 __restore_rt (libpthread.so.0 +
		    0x13140)
                #3  0x00007f87f89a5122 __GI___regexec (libc.so.6 +
		    0xdf122)
                #4  0x000055d7f198b4a7 aspath_filter_exclude_acl (bgpd +
		    0x2054a7)
                #5  0x000055d7f1902187 route_set_aspath_exclude (bgpd +
		    0x17c187)
                #6  0x00007f87f8ce54b0 route_map_apply_ext (libfrr.so.0
		    + 0xe54b0)
                #7  0x000055d7f18da925 bgp_input_modifier (bgpd +
		    0x154925)
                #8  0x000055d7f18e0647 bgp_update (bgpd + 0x15a647)
                #9  0x000055d7f18e4772 bgp_nlri_parse_ip (bgpd +
		    0x15e772)
                #10 0x000055d7f18c38ae bgp_nlri_parse (bgpd + 0x13d8ae)
                #11 0x000055d7f18c6b7a bgp_update_receive (bgpd +
		    0x140b7a)
                #12 0x000055d7f18c8ff3 bgp_process_packet (bgpd +
		    0x142ff3)
                #13 0x00007f87f8d0dce0 thread_call (libfrr.so.0 +
		    0x10dce0)
                #14 0x00007f87f8cacb28 frr_run (libfrr.so.0 + 0xacb28)
                #15 0x000055d7f18435da main (bgpd + 0xbd5da)
                #16 0x00007f87f88e9d0a __libc_start_main (libc.so.6 +
		    0x23d0a)
                #17 0x000055d7f18415fa _start (bgpd + 0xbb5fa)

analysis

crash is due to the fact that there were always a pointer from
as-path exclude to deleted as-path access list.

fix
we add a backpointer mechanism to manage the dependency beetween
as-path access-list  and aspath exclude.

Signed-off-by: Francois Dumontet <[email protected]>
rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@cscarpitta
bgpd: Fix SRv6 memory leaks spotted by ASAN
65e0111 
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 <[email protected]>
rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@louis-6wind
isisd: fix heap-after-free with prefix sid
e697de5 
> ==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 <[email protected]>
rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@louis-6wind
isisd: fix display crash srv6 sid structure in json
c06fb90 
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 <[email protected]>
rzalamena pushed a commit that referenced this pull request Jul 25, 2024
@zhou-run
isisd: fix crash when calculating the neighbor spanning tree based on…
e905177 
… 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 <[email protected]>
rzalamena pushed a commit that referenced this pull request Aug 7, 2024
@louis-6wind
pimd: fix crash on non-existent interface
6952bea 
Fix the following crash when pim options are (un)configured on an
non-existent interface.

> r1(config)# int fgljdsf
> r1(config-if)# no ip pim unicast-bsm
> vtysh: error reading from pimd: Connection reset by peer (104)Warning: closing connection to pimd because of an I/O error!

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f70c8f32249 in core_handler (signo=11, siginfo=0x7fffff88e4f0, context=0x7fffff88e3c0) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> #4  0x00007f70c8efdcb5 in nb_callback_modify (context=0x556d00032b60, nb_node=0x556cffeeb9b0, event=NB_EV_APPLY, dnode=0x556d00031670, resource=0x556d00032b48, errmsg=0x7fffff88f710 "", errmsg_len=8192)
>     at lib/northbound.c:1538
> #5  0x00007f70c8efe949 in nb_callback_configuration (context=0x556d00032b60, event=NB_EV_APPLY, change=0x556d00032b10, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1888
> #6  0x00007f70c8efee82 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x556d00032b60, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:2016
> #7  0x00007f70c8efd658 in nb_candidate_commit_apply (transaction=0x556d00032b60, save_transaction=true, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1356
> #8  0x00007f70c8efd78e in nb_candidate_commit (context=..., candidate=0x556cffeb0e80, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1389
> #9  0x00007f70c8f03e58 in nb_cli_classic_commit (vty=0x556d00025a80) at lib/northbound_cli.c:51
> #10 0x00007f70c8f043f8 in nb_cli_apply_changes_internal (vty=0x556d00025a80,
>     xpath_base=0x7fffff893bb0 "/frr-interface:lib/interface[name='fgljdsf']/frr-pim:pim/address-family[address-family='frr-routing:ipv4']", clear_pending=false) at lib/northbound_cli.c:178
> #11 0x00007f70c8f0475d in nb_cli_apply_changes (vty=0x556d00025a80, xpath_base_fmt=0x556cfdde9fe0 "./frr-pim:pim/address-family[address-family='%s']") at lib/northbound_cli.c:234
> #12 0x0000556cfdd8298f in pim_process_no_unicast_bsm_cmd (vty=0x556d00025a80) at pimd/pim_cmd_common.c:3493
> #13 0x0000556cfddcf782 in no_ip_pim_ucast_bsm (self=0x556cfde40b20 <no_ip_pim_ucast_bsm_cmd>, vty=0x556d00025a80, argc=4, argv=0x556d00031500) at pimd/pim_cmd.c:4950
> #14 0x00007f70c8e942f0 in cmd_execute_command_real (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, up_level=0) at lib/command.c:1002
> #15 0x00007f70c8e94451 in cmd_execute_command (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, vtysh=0) at lib/command.c:1061
> #16 0x00007f70c8e9499f in cmd_execute (vty=0x556d00025a80, cmd=0x556d00030320 "no ip pim unicast-bsm", matched=0x0, vtysh=0) at lib/command.c:1227
> #17 0x00007f70c8f51e44 in vty_command (vty=0x556d00025a80, buf=0x556d00030320 "no ip pim unicast-bsm") at lib/vty.c:616
> #18 0x00007f70c8f53bdd in vty_execute (vty=0x556d00025a80) at lib/vty.c:1379
> #19 0x00007f70c8f55d59 in vtysh_read (thread=0x7fffff896600) at lib/vty.c:2374
> #20 0x00007f70c8f4b209 in event_call (thread=0x7fffff896600) at lib/event.c:2011
> #21 0x00007f70c8ed109e in frr_run (master=0x556cffdb4ea0) at lib/libfrr.c:1217
> #22 0x0000556cfdddec12 in main (argc=2, argv=0x7fffff896828, envp=0x7fffff896840) at pimd/pim_main.c:165
> (gdb) f 3
> #3  0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> 1910			pim_ifp->ucast_bsm_accept =
> (gdb) list
> 1905		case NB_EV_ABORT:
> 1906			break;
> 1907		case NB_EV_APPLY:
> 1908			ifp = nb_running_get_entry(args->dnode, NULL, true);
> 1909			pim_ifp = ifp->info;
> 1910			pim_ifp->ucast_bsm_accept =
> 1911				yang_dnode_get_bool(args->dnode, NULL);
> 1912
> 1913			break;
> 1914		}
> (gdb) p pim_ifp
> $1 = (struct pim_interface *) 0x0

Fixes: 3bb513c ("lib: adapt to version 2 of libyang")
Signed-off-by: Louis Scalbert <[email protected]>
rzalamena pushed a commit that referenced this pull request Aug 7, 2024
@ton31337
bgpd: Use bgp_session_reset_safe() for GR update all peers
9c710ee 
It might cause this use-after-free:

```
==6523==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300058d720 at pc 0x55f3ab62ab1f bp 0x7ffe5b95a0d0 sp 0x7ffe5b95a0c8
READ of size 8 at 0x60300058d720 thread T0
    #0 0x55f3ab62ab1e in bgp_gr_update_mode_of_all_peers bgpd/bgp_fsm.c:2729
    #1 0x55f3ab62ab1e in bgp_gr_update_all bgpd/bgp_fsm.c:2779
    #2 0x55f3ab73557e in bgp_inst_gr_config_vty bgpd/bgp_vty.c:3037
    #3 0x55f3ab74db69 in bgp_graceful_restart bgpd/bgp_vty.c:3130
    #4 0x7fc5539a9584 in cmd_execute_command_real lib/command.c:1002
    #5 0x7fc5539a98a3 in cmd_execute_command lib/command.c:1061
    #6 0x7fc5539a9dcf in cmd_execute lib/command.c:1227
    #7 0x7fc553ae493f in vty_command lib/vty.c:616
    #8 0x7fc553ae4e92 in vty_execute lib/vty.c:1379
    #9 0x7fc553aedd34 in vtysh_read lib/vty.c:2374
    #10 0x7fc553ad8a64 in event_call lib/event.c:1995
    #11 0x7fc553a0c429 in frr_run lib/libfrr.c:1232
    #12 0x55f3ab57b78d in main bgpd/bgp_main.c:555
    #13 0x7fc55342d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x7fc55342d304 in __libc_start_main_impl ../csu/libc-start.c:360
    #15 0x55f3ab5799a0 in _start (/usr/lib/frr/bgpd+0x2e19a0)

0x60300058d720 is located 16 bytes inside of 24-byte region [0x60300058d710,0x60300058d728)
freed by thread T0 here:
    #0 0x7fc553eb76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0x7fc553a2b713 in qfree lib/memory.c:130
    #2 0x7fc553a0e50d in listnode_free lib/linklist.c:81
    #3 0x7fc553a0e50d in list_delete_node lib/linklist.c:379
    #4 0x55f3ab7ae353 in peer_delete bgpd/bgpd.c:2796
    #5 0x55f3ab7ae91f in bgp_session_reset bgpd/bgpd.c:141
    #6 0x55f3ab62ab17 in bgp_gr_update_mode_of_all_peers bgpd/bgp_fsm.c:2752
    #7 0x55f3ab62ab17 in bgp_gr_update_all bgpd/bgp_fsm.c:2779
    #8 0x55f3ab73557e in bgp_inst_gr_config_vty bgpd/bgp_vty.c:3037
    #9 0x55f3ab74db69 in bgp_graceful_restart bgpd/bgp_vty.c:3130
    #10 0x7fc5539a9584 in cmd_execute_command_real lib/command.c:1002
    #11 0x7fc5539a98a3 in cmd_execute_command lib/command.c:1061
    #12 0x7fc5539a9dcf in cmd_execute lib/command.c:1227
    #13 0x7fc553ae493f in vty_command lib/vty.c:616
    #14 0x7fc553ae4e92 in vty_execute lib/vty.c:1379
    #15 0x7fc553aedd34 in vtysh_read lib/vty.c:2374
    #16 0x7fc553ad8a64 in event_call lib/event.c:1995
    #17 0x7fc553a0c429 in frr_run lib/libfrr.c:1232
    #18 0x55f3ab57b78d in main bgpd/bgp_main.c:555
    #19 0x7fc55342d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

previously allocated by thread T0 here:
    #0 0x7fc553eb83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x7fc553a2ae20 in qcalloc lib/memory.c:105
    #2 0x7fc553a0d056 in listnode_new lib/linklist.c:71
    #3 0x7fc553a0d85b in listnode_add_sort lib/linklist.c:197
    #4 0x55f3ab7baec4 in peer_create bgpd/bgpd.c:1996
    #5 0x55f3ab65be8b in bgp_accept bgpd/bgp_network.c:604
    #6 0x7fc553ad8a64 in event_call lib/event.c:1995
    #7 0x7fc553a0c429 in frr_run lib/libfrr.c:1232
    #8 0x55f3ab57b78d in main bgpd/bgp_main.c:555
    #9 0x7fc55342d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```

Signed-off-by: Donatas Abraitis <[email protected]>
rzalamena pushed a commit that referenced this pull request Aug 14, 2024
@zhou-run @mergify
isisd: fix crash when calculating the neighbor spanning tree based on…
f727c63 
… 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 <[email protected]>
(cherry picked from commit e905177)
rzalamena pushed a commit that referenced this pull request Aug 14, 2024
@louis-6wind @mergify
pimd: fix crash on non-existent interface
4e652de 
Fix the following crash when pim options are (un)configured on an
non-existent interface.

> r1(config)# int fgljdsf
> r1(config-if)# no ip pim unicast-bsm
> vtysh: error reading from pimd: Connection reset by peer (104)Warning: closing connection to pimd because of an I/O error!

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f70c8f32249 in core_handler (signo=11, siginfo=0x7fffff88e4f0, context=0x7fffff88e3c0) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> #4  0x00007f70c8efdcb5 in nb_callback_modify (context=0x556d00032b60, nb_node=0x556cffeeb9b0, event=NB_EV_APPLY, dnode=0x556d00031670, resource=0x556d00032b48, errmsg=0x7fffff88f710 "", errmsg_len=8192)
>     at lib/northbound.c:1538
> #5  0x00007f70c8efe949 in nb_callback_configuration (context=0x556d00032b60, event=NB_EV_APPLY, change=0x556d00032b10, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1888
> #6  0x00007f70c8efee82 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x556d00032b60, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:2016
> #7  0x00007f70c8efd658 in nb_candidate_commit_apply (transaction=0x556d00032b60, save_transaction=true, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1356
> #8  0x00007f70c8efd78e in nb_candidate_commit (context=..., candidate=0x556cffeb0e80, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1389
> #9  0x00007f70c8f03e58 in nb_cli_classic_commit (vty=0x556d00025a80) at lib/northbound_cli.c:51
> #10 0x00007f70c8f043f8 in nb_cli_apply_changes_internal (vty=0x556d00025a80,
>     xpath_base=0x7fffff893bb0 "/frr-interface:lib/interface[name='fgljdsf']/frr-pim:pim/address-family[address-family='frr-routing:ipv4']", clear_pending=false) at lib/northbound_cli.c:178
> #11 0x00007f70c8f0475d in nb_cli_apply_changes (vty=0x556d00025a80, xpath_base_fmt=0x556cfdde9fe0 "./frr-pim:pim/address-family[address-family='%s']") at lib/northbound_cli.c:234
> #12 0x0000556cfdd8298f in pim_process_no_unicast_bsm_cmd (vty=0x556d00025a80) at pimd/pim_cmd_common.c:3493
> #13 0x0000556cfddcf782 in no_ip_pim_ucast_bsm (self=0x556cfde40b20 <no_ip_pim_ucast_bsm_cmd>, vty=0x556d00025a80, argc=4, argv=0x556d00031500) at pimd/pim_cmd.c:4950
> #14 0x00007f70c8e942f0 in cmd_execute_command_real (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, up_level=0) at lib/command.c:1002
> #15 0x00007f70c8e94451 in cmd_execute_command (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, vtysh=0) at lib/command.c:1061
> #16 0x00007f70c8e9499f in cmd_execute (vty=0x556d00025a80, cmd=0x556d00030320 "no ip pim unicast-bsm", matched=0x0, vtysh=0) at lib/command.c:1227
> #17 0x00007f70c8f51e44 in vty_command (vty=0x556d00025a80, buf=0x556d00030320 "no ip pim unicast-bsm") at lib/vty.c:616
> #18 0x00007f70c8f53bdd in vty_execute (vty=0x556d00025a80) at lib/vty.c:1379
> #19 0x00007f70c8f55d59 in vtysh_read (thread=0x7fffff896600) at lib/vty.c:2374
> #20 0x00007f70c8f4b209 in event_call (thread=0x7fffff896600) at lib/event.c:2011
> #21 0x00007f70c8ed109e in frr_run (master=0x556cffdb4ea0) at lib/libfrr.c:1217
> #22 0x0000556cfdddec12 in main (argc=2, argv=0x7fffff896828, envp=0x7fffff896840) at pimd/pim_main.c:165
> (gdb) f 3
> #3  0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> 1910			pim_ifp->ucast_bsm_accept =
> (gdb) list
> 1905		case NB_EV_ABORT:
> 1906			break;
> 1907		case NB_EV_APPLY:
> 1908			ifp = nb_running_get_entry(args->dnode, NULL, true);
> 1909			pim_ifp = ifp->info;
> 1910			pim_ifp->ucast_bsm_accept =
> 1911				yang_dnode_get_bool(args->dnode, NULL);
> 1912
> 1913			break;
> 1914		}
> (gdb) p pim_ifp
> $1 = (struct pim_interface *) 0x0

Fixes: 3bb513c ("lib: adapt to version 2 of libyang")
Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit 6952bea)
rzalamena pushed a commit that referenced this pull request Aug 15, 2024
@pguibert6WIND
bgpd: fix memory type for static->prd_pretty
64594f8 
A crash happens when executing the following command:

> ubuntu2204hwe# conf
> ubuntu2204hwe(config)# router bgp 65500
> ubuntu2204hwe(config-router)#  !
> ubuntu2204hwe(config-router)#  address-family ipv4 unicast
> ubuntu2204hwe(config-router-af)#   sid vpn export auto
> ubuntu2204hwe(config-router-af)#  exit-address-family
> ubuntu2204hwe(config-router)#  !
> ubuntu2204hwe(config-router)#  address-family ipv4 vpn
> ubuntu2204hwe(config-router-af)#   network 4.4.4.4/32 rd 55:55 label 556
> ubuntu2204hwe(config-router-af)#   network 5.5.5.5/32 rd 662:33 label 232
> ubuntu2204hwe(config-router-af)#  exit-address-family
> ubuntu2204hwe(config-router)# exit
> ubuntu2204hwe(config)# !
> ubuntu2204hwe(config)# no router bgp

The crash analysis indicates a memory item has been freed.

> #6  0x000076066a629c15 in mt_count_free (mt=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0)
>     at lib/memory.c:73
> #7  mt_count_free (ptr=0x60200038b4f0, mt=0x56b57be85e00 <MTYPE_BGP_NAME>) at lib/memory.c:69
> #8  qfree (mt=mt@entry=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0) at lib/memory.c:129
> #9  0x000056b57bb09ce9 in bgp_free (bgp=<optimized out>) at bgpd/bgpd.c:4120
> #10 0x000056b57bb0aa73 in bgp_unlock (bgp=<optimized out>) at ./bgpd/bgpd.h:2513
> #11 peer_free (peer=0x62a000000200) at bgpd/bgpd.c:1313
> #12 0x000056b57bb0aca8 in peer_unlock_with_caller (name=<optimized out>, peer=<optimized out>)
>     at bgpd/bgpd.c:1344
> #13 0x000076066a6dbb2c in event_call (thread=thread@entry=0x7ffc8cae1d60) at lib/event.c:2011
> #14 0x000076066a60aa88 in frr_run (master=0x613000000040) at lib/libfrr.c:1214
> #15 0x000056b57b8b2c44 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:543

Actually, the BGP_NAME item has not been used at allocation for
static->prd_pretty, and this results in reaching 0 quicker at bgp
deletion.

Fix this by reassigning MTYPE_BGP_NAME to prd_pretty.

Fixes: 16600df2c4f4 ("bgpd: fix show run of network route-distinguisher")

Signed-off-by: Philippe Guibert <[email protected]>
rzalamena pushed a commit that referenced this pull request Aug 15, 2024
@pguibert6WIND @mergify
bgpd: fix memory type for static->prd_pretty
fd66383 
A crash happens when executing the following command:

> ubuntu2204hwe# conf
> ubuntu2204hwe(config)# router bgp 65500
> ubuntu2204hwe(config-router)#  !
> ubuntu2204hwe(config-router)#  address-family ipv4 unicast
> ubuntu2204hwe(config-router-af)#   sid vpn export auto
> ubuntu2204hwe(config-router-af)#  exit-address-family
> ubuntu2204hwe(config-router)#  !
> ubuntu2204hwe(config-router)#  address-family ipv4 vpn
> ubuntu2204hwe(config-router-af)#   network 4.4.4.4/32 rd 55:55 label 556
> ubuntu2204hwe(config-router-af)#   network 5.5.5.5/32 rd 662:33 label 232
> ubuntu2204hwe(config-router-af)#  exit-address-family
> ubuntu2204hwe(config-router)# exit
> ubuntu2204hwe(config)# !
> ubuntu2204hwe(config)# no router bgp

The crash analysis indicates a memory item has been freed.

> #6  0x000076066a629c15 in mt_count_free (mt=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0)
>     at lib/memory.c:73
> #7  mt_count_free (ptr=0x60200038b4f0, mt=0x56b57be85e00 <MTYPE_BGP_NAME>) at lib/memory.c:69
> #8  qfree (mt=mt@entry=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0) at lib/memory.c:129
> #9  0x000056b57bb09ce9 in bgp_free (bgp=<optimized out>) at bgpd/bgpd.c:4120
> #10 0x000056b57bb0aa73 in bgp_unlock (bgp=<optimized out>) at ./bgpd/bgpd.h:2513
> #11 peer_free (peer=0x62a000000200) at bgpd/bgpd.c:1313
> #12 0x000056b57bb0aca8 in peer_unlock_with_caller (name=<optimized out>, peer=<optimized out>)
>     at bgpd/bgpd.c:1344
> #13 0x000076066a6dbb2c in event_call (thread=thread@entry=0x7ffc8cae1d60) at lib/event.c:2011
> #14 0x000076066a60aa88 in frr_run (master=0x613000000040) at lib/libfrr.c:1214
> #15 0x000056b57b8b2c44 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:543

Actually, the BGP_NAME item has not been used at allocation for
static->prd_pretty, and this results in reaching 0 quicker at bgp
deletion.

Fix this by reassigning MTYPE_BGP_NAME to prd_pretty.

Fixes: 16600df2c4f4 ("bgpd: fix show run of network route-distinguisher")

Signed-off-by: Philippe Guibert <[email protected]>
(cherry picked from commit 64594f8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mwinter-osr @RodrigoMNardi