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

frr: Modify doc to talk about cli #7

Merged
merged 1 commit into from
Dec 20, 2016

Conversation

donaldsharp
Copy link
Member

Add some verbiage to the COMMUNITY.md to talk
about the philosophy of what cli should do.

Signed-off-by: Donald Sharp sharpd@cumulusnetworks.com

Add some verbiage to the COMMUNITY.md to talk
about the philosophy of what cli should do.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

CLI's are a complicated ugly beast. Additions or changes to the CLI
should use a DEFUN to encapsulate one setting as much as is possible.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it super explicit, perhaps:

"Unrelated CLI definitions should not be bundled together."

@eqvinox eqvinox merged commit e179b7b into FRRouting:stable/2.0 Dec 20, 2016
@donaldsharp donaldsharp deleted the community branch May 17, 2017 12:01
@louberger louberger mentioned this pull request Dec 13, 2017
cfra referenced this pull request in opensourcerouting/frr Nov 29, 2018
rwestphal referenced this pull request in opensourcerouting/frr Feb 21, 2019
If path->net is NULL in the bgp_path_info_free() function, then
bgpd would crash in bgp_addpath_free_info_data() with the following
backtrace:

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

Add a null-check protection to fix this problem.

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

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

Add a null-check protection to fix this problem.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 9, 2024
The following causes a isisd crash.

> # cat config
> affinity-map green bit-position 0
> router isis 1
>  flex-algo 129
>   affinity exclude-any green
> # vtysh -f config

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f650cd32756 in core_handler (signo=6, siginfo=0x7ffc56f93070, context=0x7ffc56f92f40) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#4  0x00007f650c91c537 in __GI_abort () at abort.c:79
> FRRouting#5  0x00007f650cd007c9 in nb_running_get_entry_worker (dnode=0x0, xpath=0x0, abort_if_not_found=true, rec_search=true) at lib/northbound.c:2531
> FRRouting#6  0x00007f650cd007f9 in nb_running_get_entry (dnode=0x55d9ad406e00, xpath=0x0, abort_if_not_found=true) at lib/northbound.c:2537
> FRRouting#7  0x000055d9ab302248 in isis_instance_flex_algo_affinity_set (args=0x7ffc56f947a0, type=2) at isisd/isis_nb_config.c:2998
> FRRouting#8  0x000055d9ab3027c0 in isis_instance_flex_algo_affinity_exclude_any_create (args=0x7ffc56f947a0) at isisd/isis_nb_config.c:3155
> FRRouting#9  0x00007f650ccfe284 in nb_callback_create (context=0x7ffc56f94d20, nb_node=0x55d9ad28b540, event=NB_EV_VALIDATE, dnode=0x55d9ad406e00, resource=0x0, errmsg=0x7ffc56f94de0 "",
>     errmsg_len=8192) at lib/northbound.c:1487
> FRRouting#10 0x00007f650ccff067 in nb_callback_configuration (context=0x7ffc56f94d20, event=NB_EV_VALIDATE, change=0x55d9ad406d40, errmsg=0x7ffc56f94de0 "", errmsg_len=8192) at lib/northbound.c:1884
> FRRouting#11 0x00007f650ccfda31 in nb_candidate_validate_code (context=0x7ffc56f94d20, candidate=0x55d9ad20d710, changes=0x7ffc56f94d38, errmsg=0x7ffc56f94de0 "", errmsg_len=8192)
>     at lib/northbound.c:1246
> FRRouting#12 0x00007f650ccfdc67 in nb_candidate_commit_prepare (context=..., candidate=0x55d9ad20d710, comment=0x0, transaction=0x7ffc56f94da0, skip_validate=false, ignore_zero_change=false,
>     errmsg=0x7ffc56f94de0 "", errmsg_len=8192) at lib/northbound.c:1317
> FRRouting#13 0x00007f650ccfdec4 in nb_candidate_commit (context=..., candidate=0x55d9ad20d710, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffc56f94de0 "", errmsg_len=8192)
>     at lib/northbound.c:1381
> FRRouting#14 0x00007f650cd045ba in nb_cli_classic_commit (vty=0x55d9ad3f7490) at lib/northbound_cli.c:57
> FRRouting#15 0x00007f650cd04749 in nb_cli_pending_commit_check (vty=0x55d9ad3f7490) at lib/northbound_cli.c:96
> FRRouting#16 0x00007f650cc94340 in cmd_execute_command_real (vline=0x55d9ad3eea10, vty=0x55d9ad3f7490, cmd=0x0, up_level=0) at lib/command.c:1000
> FRRouting#17 0x00007f650cc94599 in cmd_execute_command (vline=0x55d9ad3eea10, vty=0x55d9ad3f7490, cmd=0x0, vtysh=0) at lib/command.c:1080
> FRRouting#18 0x00007f650cc94a0c in cmd_execute (vty=0x55d9ad3f7490, cmd=0x55d9ad401d30 "XFRR_end_configuration", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#19 0x00007f650cd523a4 in vty_command (vty=0x55d9ad3f7490, buf=0x55d9ad401d30 "XFRR_end_configuration") at lib/vty.c:625
> FRRouting#20 0x00007f650cd5413d in vty_execute (vty=0x55d9ad3f7490) at lib/vty.c:1388
> FRRouting#21 0x00007f650cd56353 in vtysh_read (thread=0x7ffc56f99370) at lib/vty.c:2400
> FRRouting#22 0x00007f650cd4b6fd in event_call (thread=0x7ffc56f99370) at lib/event.c:1996
> FRRouting#23 0x00007f650ccd1365 in frr_run (master=0x55d9ad103cf0) at lib/libfrr.c:1231
> FRRouting#24 0x000055d9ab29036e in main (argc=2, argv=0x7ffc56f99598, envp=0x7ffc56f995b0) at isisd/isis_main.c:354

Configuring the same in vtysh configure interactive mode works properly.
When using "vtysh -f", the northbound compatible configuration is
committed together whereas, in interactive mode, it committed line by
line. In the first situation, in validation state nb_running_get_entry()
fails because the area not yet in running.

Do not use nb_running_get_entry() northbound validation state.

Fixes: 893882e ("isisd: add isis flex-algo configuration backend")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 10, 2024
Fix a crash when modifying a route-map with set as-path exclude without
as-path-access-list:

> router(config)# route-map routemaptest deny 1
> router(config-route-map)# set as-path exclude 33 34 35
> router(config-route-map)# set as-path exclude as-path-access-list test

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fb3959327de in core_handler (signo=11, siginfo=0x7ffd122da530, context=0x7ffd122da400) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  0x000055ab2762a1bd in as_list_list_del (h=0x55ab27897680 <as_exclude_list_orphan>, item=0x55ab28204e20) at ./bgpd/bgp_aspath.h:77
> FRRouting#4  0x000055ab2762d1a8 in as_exclude_remove_orphan (ase=0x55ab28204e20) at bgpd/bgp_aspath.c:1574
> FRRouting#5  0x000055ab27550538 in route_aspath_exclude_free (rule=0x55ab28204e20) at bgpd/bgp_routemap.c:2366
> FRRouting#6  0x00007fb39591f00c in route_map_rule_delete (list=0x55ab28203498, rule=0x55ab28204170) at lib/routemap.c:1357
> FRRouting#7  0x00007fb39591f87c in route_map_add_set (index=0x55ab28203460, set_name=0x55ab276ad2aa "as-path exclude", set_arg=0x55ab281e4f70 "as-path-access-list test") at lib/routemap.c:1674
> FRRouting#8  0x00007fb39591d3f3 in generic_set_add (index=0x55ab28203460, command=0x55ab276ad2aa "as-path exclude", arg=0x55ab281e4f70 "as-path-access-list test", errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/routemap.c:533
> FRRouting#9  0x000055ab2755e78e in lib_route_map_entry_set_action_rmap_set_action_exclude_as_path_modify (args=0x7ffd122db290) at bgpd/bgp_routemap_nb_config.c:2427
> FRRouting#10 0x00007fb3958fe417 in nb_callback_modify (context=0x55ab28205aa0, nb_node=0x55ab27cb31e0, event=NB_EV_APPLY, dnode=0x55ab28202690, resource=0x55ab27c32148, errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/northbound.c:1538
> FRRouting#11 0x00007fb3958ff0ab in nb_callback_configuration (context=0x55ab28205aa0, event=NB_EV_APPLY, change=0x55ab27c32110, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:1888
> FRRouting#12 0x00007fb3958ff5e4 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x55ab28205aa0, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:2016
> FRRouting#13 0x00007fb3958fddba in nb_candidate_commit_apply (transaction=0x55ab28205aa0, save_transaction=true, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1356
> FRRouting#14 0x00007fb3958fdef0 in nb_candidate_commit (context=..., candidate=0x55ab27c2c9a0, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1389
> FRRouting#15 0x00007fb3959045ba in nb_cli_classic_commit (vty=0x55ab281f6680) at lib/northbound_cli.c:57
> FRRouting#16 0x00007fb395904b5a in nb_cli_apply_changes_internal (vty=0x55ab281f6680, xpath_base=0x7ffd122dfd10 "/frr-route-map:lib/route-map[name='routemaptest']/entry[sequence='1']",
>     clear_pending=false) at lib/northbound_cli.c:184
> FRRouting#17 0x00007fb395904ebf in nb_cli_apply_changes (vty=0x55ab281f6680, xpath_base_fmt=0x0) at lib/northbound_cli.c:240
> --Type <RET> for more, q to quit, c to continue without paging--
> FRRouting#18 0x000055ab27557d2e in set_aspath_exclude_access_list_magic (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80,
>     as_path_filter_name=0x55ab28202040 "test") at bgpd/bgp_routemap.c:6397
> FRRouting#19 0x000055ab2754bdea in set_aspath_exclude_access_list (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80)
>     at ./bgpd/bgp_routemap_clippy.c:856
> FRRouting#20 0x00007fb39589435d in cmd_execute_command_real (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, up_level=0) at lib/command.c:1003
> FRRouting#21 0x00007fb3958944be in cmd_execute_command (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, vtysh=0) at lib/command.c:1062
> FRRouting#22 0x00007fb395894a0c in cmd_execute (vty=0x55ab281f6680, cmd=0x55ab28200f20 "set as-path exclude as-path-access-list test", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#23 0x00007fb39595242c in vty_command (vty=0x55ab281f6680, buf=0x55ab28200f20 "set as-path exclude as-path-access-list test") at lib/vty.c:625
> FRRouting#24 0x00007fb3959541c5 in vty_execute (vty=0x55ab281f6680) at lib/vty.c:1388
> FRRouting#25 0x00007fb3959563db in vtysh_read (thread=0x7ffd122e2bb0) at lib/vty.c:2400
> FRRouting#26 0x00007fb39594b785 in event_call (thread=0x7ffd122e2bb0) at lib/event.c:1996
> FRRouting#27 0x00007fb3958d1365 in frr_run (master=0x55ab27b56d70) at lib/libfrr.c:1231
> FRRouting#28 0x000055ab2747f1cc in main (argc=3, argv=0x7ffd122e2e08) at bgpd/bgp_main.c:555

Fixes: 094dcc3 ("bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp pushed a commit that referenced this pull request Sep 11, 2024
Fix a crash when modifying a route-map with set as-path exclude without
as-path-access-list:

> router(config)# route-map routemaptest deny 1
> router(config-route-map)# set as-path exclude 33 34 35
> router(config-route-map)# set as-path exclude as-path-access-list test

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fb3959327de in core_handler (signo=11, siginfo=0x7ffd122da530, context=0x7ffd122da400) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  0x000055ab2762a1bd in as_list_list_del (h=0x55ab27897680 <as_exclude_list_orphan>, item=0x55ab28204e20) at ./bgpd/bgp_aspath.h:77
> #4  0x000055ab2762d1a8 in as_exclude_remove_orphan (ase=0x55ab28204e20) at bgpd/bgp_aspath.c:1574
> #5  0x000055ab27550538 in route_aspath_exclude_free (rule=0x55ab28204e20) at bgpd/bgp_routemap.c:2366
> #6  0x00007fb39591f00c in route_map_rule_delete (list=0x55ab28203498, rule=0x55ab28204170) at lib/routemap.c:1357
> #7  0x00007fb39591f87c in route_map_add_set (index=0x55ab28203460, set_name=0x55ab276ad2aa "as-path exclude", set_arg=0x55ab281e4f70 "as-path-access-list test") at lib/routemap.c:1674
> #8  0x00007fb39591d3f3 in generic_set_add (index=0x55ab28203460, command=0x55ab276ad2aa "as-path exclude", arg=0x55ab281e4f70 "as-path-access-list test", errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/routemap.c:533
> #9  0x000055ab2755e78e in lib_route_map_entry_set_action_rmap_set_action_exclude_as_path_modify (args=0x7ffd122db290) at bgpd/bgp_routemap_nb_config.c:2427
> #10 0x00007fb3958fe417 in nb_callback_modify (context=0x55ab28205aa0, nb_node=0x55ab27cb31e0, event=NB_EV_APPLY, dnode=0x55ab28202690, resource=0x55ab27c32148, errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/northbound.c:1538
> #11 0x00007fb3958ff0ab in nb_callback_configuration (context=0x55ab28205aa0, event=NB_EV_APPLY, change=0x55ab27c32110, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:1888
> #12 0x00007fb3958ff5e4 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x55ab28205aa0, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:2016
> #13 0x00007fb3958fddba in nb_candidate_commit_apply (transaction=0x55ab28205aa0, save_transaction=true, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1356
> #14 0x00007fb3958fdef0 in nb_candidate_commit (context=..., candidate=0x55ab27c2c9a0, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1389
> #15 0x00007fb3959045ba in nb_cli_classic_commit (vty=0x55ab281f6680) at lib/northbound_cli.c:57
> #16 0x00007fb395904b5a in nb_cli_apply_changes_internal (vty=0x55ab281f6680, xpath_base=0x7ffd122dfd10 "/frr-route-map:lib/route-map[name='routemaptest']/entry[sequence='1']",
>     clear_pending=false) at lib/northbound_cli.c:184
> #17 0x00007fb395904ebf in nb_cli_apply_changes (vty=0x55ab281f6680, xpath_base_fmt=0x0) at lib/northbound_cli.c:240
> --Type <RET> for more, q to quit, c to continue without paging--
> #18 0x000055ab27557d2e in set_aspath_exclude_access_list_magic (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80,
>     as_path_filter_name=0x55ab28202040 "test") at bgpd/bgp_routemap.c:6397
> #19 0x000055ab2754bdea in set_aspath_exclude_access_list (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80)
>     at ./bgpd/bgp_routemap_clippy.c:856
> #20 0x00007fb39589435d in cmd_execute_command_real (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, up_level=0) at lib/command.c:1003
> #21 0x00007fb3958944be in cmd_execute_command (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, vtysh=0) at lib/command.c:1062
> #22 0x00007fb395894a0c in cmd_execute (vty=0x55ab281f6680, cmd=0x55ab28200f20 "set as-path exclude as-path-access-list test", matched=0x0, vtysh=0) at lib/command.c:1228
> #23 0x00007fb39595242c in vty_command (vty=0x55ab281f6680, buf=0x55ab28200f20 "set as-path exclude as-path-access-list test") at lib/vty.c:625
> #24 0x00007fb3959541c5 in vty_execute (vty=0x55ab281f6680) at lib/vty.c:1388
> #25 0x00007fb3959563db in vtysh_read (thread=0x7ffd122e2bb0) at lib/vty.c:2400
> #26 0x00007fb39594b785 in event_call (thread=0x7ffd122e2bb0) at lib/event.c:1996
> #27 0x00007fb3958d1365 in frr_run (master=0x55ab27b56d70) at lib/libfrr.c:1231
> #28 0x000055ab2747f1cc in main (argc=3, argv=0x7ffd122e2e08) at bgpd/bgp_main.c:555

Fixes: 094dcc3 ("bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp pushed a commit that referenced this pull request Sep 11, 2024
Fix a crash when modifying a route-map with set as-path exclude without
as-path-access-list:

> router(config)# route-map routemaptest deny 1
> router(config-route-map)# set as-path exclude 33 34 35
> router(config-route-map)# set as-path exclude as-path-access-list test

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fb3959327de in core_handler (signo=11, siginfo=0x7ffd122da530, context=0x7ffd122da400) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  0x000055ab2762a1bd in as_list_list_del (h=0x55ab27897680 <as_exclude_list_orphan>, item=0x55ab28204e20) at ./bgpd/bgp_aspath.h:77
> #4  0x000055ab2762d1a8 in as_exclude_remove_orphan (ase=0x55ab28204e20) at bgpd/bgp_aspath.c:1574
> #5  0x000055ab27550538 in route_aspath_exclude_free (rule=0x55ab28204e20) at bgpd/bgp_routemap.c:2366
> #6  0x00007fb39591f00c in route_map_rule_delete (list=0x55ab28203498, rule=0x55ab28204170) at lib/routemap.c:1357
> #7  0x00007fb39591f87c in route_map_add_set (index=0x55ab28203460, set_name=0x55ab276ad2aa "as-path exclude", set_arg=0x55ab281e4f70 "as-path-access-list test") at lib/routemap.c:1674
> #8  0x00007fb39591d3f3 in generic_set_add (index=0x55ab28203460, command=0x55ab276ad2aa "as-path exclude", arg=0x55ab281e4f70 "as-path-access-list test", errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/routemap.c:533
> #9  0x000055ab2755e78e in lib_route_map_entry_set_action_rmap_set_action_exclude_as_path_modify (args=0x7ffd122db290) at bgpd/bgp_routemap_nb_config.c:2427
> #10 0x00007fb3958fe417 in nb_callback_modify (context=0x55ab28205aa0, nb_node=0x55ab27cb31e0, event=NB_EV_APPLY, dnode=0x55ab28202690, resource=0x55ab27c32148, errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/northbound.c:1538
> #11 0x00007fb3958ff0ab in nb_callback_configuration (context=0x55ab28205aa0, event=NB_EV_APPLY, change=0x55ab27c32110, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:1888
> #12 0x00007fb3958ff5e4 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x55ab28205aa0, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:2016
> #13 0x00007fb3958fddba in nb_candidate_commit_apply (transaction=0x55ab28205aa0, save_transaction=true, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1356
> #14 0x00007fb3958fdef0 in nb_candidate_commit (context=..., candidate=0x55ab27c2c9a0, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1389
> #15 0x00007fb3959045ba in nb_cli_classic_commit (vty=0x55ab281f6680) at lib/northbound_cli.c:57
> #16 0x00007fb395904b5a in nb_cli_apply_changes_internal (vty=0x55ab281f6680, xpath_base=0x7ffd122dfd10 "/frr-route-map:lib/route-map[name='routemaptest']/entry[sequence='1']",
>     clear_pending=false) at lib/northbound_cli.c:184
> #17 0x00007fb395904ebf in nb_cli_apply_changes (vty=0x55ab281f6680, xpath_base_fmt=0x0) at lib/northbound_cli.c:240
> --Type <RET> for more, q to quit, c to continue without paging--
> #18 0x000055ab27557d2e in set_aspath_exclude_access_list_magic (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80,
>     as_path_filter_name=0x55ab28202040 "test") at bgpd/bgp_routemap.c:6397
> #19 0x000055ab2754bdea in set_aspath_exclude_access_list (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80)
>     at ./bgpd/bgp_routemap_clippy.c:856
> #20 0x00007fb39589435d in cmd_execute_command_real (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, up_level=0) at lib/command.c:1003
> #21 0x00007fb3958944be in cmd_execute_command (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, vtysh=0) at lib/command.c:1062
> #22 0x00007fb395894a0c in cmd_execute (vty=0x55ab281f6680, cmd=0x55ab28200f20 "set as-path exclude as-path-access-list test", matched=0x0, vtysh=0) at lib/command.c:1228
> #23 0x00007fb39595242c in vty_command (vty=0x55ab281f6680, buf=0x55ab28200f20 "set as-path exclude as-path-access-list test") at lib/vty.c:625
> #24 0x00007fb3959541c5 in vty_execute (vty=0x55ab281f6680) at lib/vty.c:1388
> #25 0x00007fb3959563db in vtysh_read (thread=0x7ffd122e2bb0) at lib/vty.c:2400
> #26 0x00007fb39594b785 in event_call (thread=0x7ffd122e2bb0) at lib/event.c:1996
> #27 0x00007fb3958d1365 in frr_run (master=0x55ab27b56d70) at lib/libfrr.c:1231
> #28 0x000055ab2747f1cc in main (argc=3, argv=0x7ffd122e2e08) at bgpd/bgp_main.c:555

Fixes: 094dcc3 ("bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 12, 2024
Level 2 adjacency list is not supposed to be always set.

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f9f0353274f in core_handler (signo=6, siginfo=0x7ffe95260770, context=0x7ffe95260640) at lib/sigevent.c:258
> #2  <signal handler called>
> #3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#4  0x00007f9f0324e537 in __GI_abort () at abort.c:79
> FRRouting#5  0x00007f9f035744ea in _zlog_assert_failed (xref=0x7f9f0362c6c0 <_xref.15>, extra=0x0) at lib/zlog.c:789
> FRRouting#6  0x00007f9f034d25ee in listnode_head (list=0x0) at lib/linklist.c:316
> FRRouting#7  0x000055cd65aaa481 in lib_interface_state_isis_adjacencies_adjacency_get_next (args=0x7ffe95261730) at isisd/isis_nb_state.c:101
> FRRouting#8  0x00007f9f034feadd in nb_callback_get_next (nb_node=0x55cd673c0190, parent_list_entry=0x55cd67570d30, list_entry=0x55cd6758f8a0) at lib/northbound.c:1748
> FRRouting#9  0x00007f9f0350bf07 in __walk (ys=0x55cd675782b0, is_resume=false) at lib/northbound_oper.c:1264
> FRRouting#10 0x00007f9f0350deaa in nb_op_walk_start (ys=0x55cd675782b0) at lib/northbound_oper.c:1741
> FRRouting#11 0x00007f9f0350e079 in nb_oper_iterate_legacy (xpath=0x55cd67595c60 "/frr-interface:lib", translator=0x0, flags=0, cb=0x0, cb_arg=0x0, tree=0x7ffe952621b0) at lib/northbound_oper.c:1803
> FRRouting#12 0x00007f9f03507661 in show_yang_operational_data_magic (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0,
>     xpath=0x55cd67595c60 "/frr-interface:lib", json=0x0, xml=0x0, translator_family=0x0, with_config=0x0) at lib/northbound_cli.c:1576
> FRRouting#13 0x00007f9f035037f0 in show_yang_operational_data (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0)
>     at ./lib/northbound_cli_clippy.c:906
> FRRouting#14 0x00007f9f0349435d in cmd_execute_command_real (vline=0x55cd6758e490, vty=0x55cd675a61f0, cmd=0x0, up_level=0) at lib/command.c:1003
> FRRouting#15 0x00007f9f03494477 in cmd_execute_command (vline=0x55cd67585340, vty=0x55cd675a61f0, cmd=0x0, vtysh=0) at lib/command.c:1053
> FRRouting#16 0x00007f9f03494a0c in cmd_execute (vty=0x55cd675a61f0, cmd=0x55cd67579040 "do show yang operational-data /frr-interface:lib", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#17 0x00007f9f0355239d in vty_command (vty=0x55cd675a61f0, buf=0x55cd67579040 "do show yang operational-data /frr-interface:lib") at lib/vty.c:625
> FRRouting#18 0x00007f9f03554136 in vty_execute (vty=0x55cd675a61f0) at lib/vty.c:1388
> FRRouting#19 0x00007f9f0355634c in vtysh_read (thread=0x7ffe952647a0) at lib/vty.c:2400
> FRRouting#20 0x00007f9f0354b6f6 in event_call (thread=0x7ffe952647a0) at lib/event.c:1996
> FRRouting#21 0x00007f9f034d1365 in frr_run (master=0x55cd67204da0) at lib/libfrr.c:1231
> FRRouting#22 0x000055cd65a3236e in main (argc=7, argv=0x7ffe952649c8, envp=0x7ffe95264a08) at isisd/isis_main.c:354

Fixes: 2a1c520 ("isisd: split northbound callbacks into multiple files")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
donaldsharp pushed a commit that referenced this pull request Sep 13, 2024
When 'no rpki' is requested and the rtrlib RPKI object was freed, bgpd
is crashing.

RPKI is configured in VRF red.

> ip l set red down
> ip l del red
> printf 'conf\n vrf red\n no rpki' | vtysh

> Core was generated by `/usr/bin/bgpd -A 127.0.0.1 -M snmp -M rpki -M bmp'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  __pthread_kill_implementation (no_tid=0, signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:44
> 44	./nptl/pthread_kill.c: No such file or directory.
> [Current thread is 1 (Thread 0x7fb401f419c0 (LWP 190226))]
> (gdb) bt
> #0  __pthread_kill_implementation (no_tid=0, signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:44
> #1  __pthread_kill_internal (signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:78
> #2  __GI___pthread_kill (threadid=140411103615424, signo=signo@entry=11) at ./nptl/pthread_kill.c:89
> #3  0x00007fb4021ad476 in __GI_raise (sig=11) at ../sysdeps/posix/raise.c:26
> #4  0x00007fb4025ce22b in core_handler (signo=11, siginfo=0x7fff831b2d70, context=0x7fff831b2c40) at lib/sigevent.c:248
> #5  <signal handler called>
> #6  rtr_mgr_remove_group (config=0x55fe8789f750, preference=11) at /build/make-pkg/output/source/DIST_RTRLIB/rtrlib/rtrlib/rtr_mgr.c:607
> #7  0x00007fb40145f518 in rpki_delete_all_cache_nodes (rpki_vrf=0x55fe8789f4f0) at bgpd/bgp_rpki.c:442
> #8  0x00007fb401463098 in no_rpki_magic (self=0x7fb40146bba0 <no_rpki_cmd>, vty=0x55fe877f5130, argc=2, argv=0x55fe877fccd0) at bgpd/bgp_rpki.c:1732
> #9  0x00007fb40145c09a in no_rpki (self=0x7fb40146bba0 <no_rpki_cmd>, vty=0x55fe877f5130, argc=2, argv=0x55fe877fccd0) at ./bgpd/bgp_rpki_clippy.c:37
> #10 0x00007fb402527abc in cmd_execute_command_real (vline=0x55fe877fd150, vty=0x55fe877f5130, cmd=0x0, up_level=0) at lib/command.c:984
> #11 0x00007fb402527c35 in cmd_execute_command (vline=0x55fe877fd150, vty=0x55fe877f5130, cmd=0x0, vtysh=0) at lib/command.c:1043
> #12 0x00007fb4025281e5 in cmd_execute (vty=0x55fe877f5130, cmd=0x55fe877fb8c0 "no rpki\n", matched=0x0, vtysh=0) at lib/command.c:1209
> #13 0x00007fb4025f0aed in vty_command (vty=0x55fe877f5130, buf=0x55fe877fb8c0 "no rpki\n") at lib/vty.c:615
> #14 0x00007fb4025f2a11 in vty_execute (vty=0x55fe877f5130) at lib/vty.c:1378
> #15 0x00007fb4025f513d in vtysh_read (thread=0x7fff831b5fa0) at lib/vty.c:2373
> #16 0x00007fb4025e9611 in event_call (thread=0x7fff831b5fa0) at lib/event.c:2011
> #17 0x00007fb402566976 in frr_run (master=0x55fe871a14a0) at lib/libfrr.c:1212
> #18 0x000055fe857829fa in main (argc=9, argv=0x7fff831b6218) at bgpd/bgp_main.c:549

Fixes: 8156765 ("bgpd: Add `no rpki` command")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
choppsv1 pushed a commit to LabNConsulting/frr that referenced this pull request Sep 14, 2024
Level 2 adjacency list is not supposed to be always set.

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f9f0353274f in core_handler (signo=6, siginfo=0x7ffe95260770, context=0x7ffe95260640) at lib/sigevent.c:258
> #2  <signal handler called>
> FRRouting#3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#4  0x00007f9f0324e537 in __GI_abort () at abort.c:79
> FRRouting#5  0x00007f9f035744ea in _zlog_assert_failed (xref=0x7f9f0362c6c0 <_xref.15>, extra=0x0) at lib/zlog.c:789
> FRRouting#6  0x00007f9f034d25ee in listnode_head (list=0x0) at lib/linklist.c:316
> FRRouting#7  0x000055cd65aaa481 in lib_interface_state_isis_adjacencies_adjacency_get_next (args=0x7ffe95261730) at isisd/isis_nb_state.c:101
> FRRouting#8  0x00007f9f034feadd in nb_callback_get_next (nb_node=0x55cd673c0190, parent_list_entry=0x55cd67570d30, list_entry=0x55cd6758f8a0) at lib/northbound.c:1748
> FRRouting#9  0x00007f9f0350bf07 in __walk (ys=0x55cd675782b0, is_resume=false) at lib/northbound_oper.c:1264
> FRRouting#10 0x00007f9f0350deaa in nb_op_walk_start (ys=0x55cd675782b0) at lib/northbound_oper.c:1741
> FRRouting#11 0x00007f9f0350e079 in nb_oper_iterate_legacy (xpath=0x55cd67595c60 "/frr-interface:lib", translator=0x0, flags=0, cb=0x0, cb_arg=0x0, tree=0x7ffe952621b0) at lib/northbound_oper.c:1803
> FRRouting#12 0x00007f9f03507661 in show_yang_operational_data_magic (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0,
>     xpath=0x55cd67595c60 "/frr-interface:lib", json=0x0, xml=0x0, translator_family=0x0, with_config=0x0) at lib/northbound_cli.c:1576
> FRRouting#13 0x00007f9f035037f0 in show_yang_operational_data (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0)
>     at ./lib/northbound_cli_clippy.c:906
> FRRouting#14 0x00007f9f0349435d in cmd_execute_command_real (vline=0x55cd6758e490, vty=0x55cd675a61f0, cmd=0x0, up_level=0) at lib/command.c:1003
> FRRouting#15 0x00007f9f03494477 in cmd_execute_command (vline=0x55cd67585340, vty=0x55cd675a61f0, cmd=0x0, vtysh=0) at lib/command.c:1053
> FRRouting#16 0x00007f9f03494a0c in cmd_execute (vty=0x55cd675a61f0, cmd=0x55cd67579040 "do show yang operational-data /frr-interface:lib", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#17 0x00007f9f0355239d in vty_command (vty=0x55cd675a61f0, buf=0x55cd67579040 "do show yang operational-data /frr-interface:lib") at lib/vty.c:625
> FRRouting#18 0x00007f9f03554136 in vty_execute (vty=0x55cd675a61f0) at lib/vty.c:1388
> FRRouting#19 0x00007f9f0355634c in vtysh_read (thread=0x7ffe952647a0) at lib/vty.c:2400
> FRRouting#20 0x00007f9f0354b6f6 in event_call (thread=0x7ffe952647a0) at lib/event.c:1996
> FRRouting#21 0x00007f9f034d1365 in frr_run (master=0x55cd67204da0) at lib/libfrr.c:1231
> FRRouting#22 0x000055cd65a3236e in main (argc=7, argv=0x7ffe952649c8, envp=0x7ffe95264a08) at isisd/isis_main.c:354

Fixes: 2a1c520 ("isisd: split northbound callbacks into multiple files")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
choppsv1 pushed a commit to LabNConsulting/frr that referenced this pull request Sep 14, 2024
Fix a crash when modifying a route-map with set as-path exclude without
as-path-access-list:

> router(config)# route-map routemaptest deny 1
> router(config-route-map)# set as-path exclude 33 34 35
> router(config-route-map)# set as-path exclude as-path-access-list test

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fb3959327de in core_handler (signo=11, siginfo=0x7ffd122da530, context=0x7ffd122da400) at lib/sigevent.c:258
> #2  <signal handler called>
> FRRouting#3  0x000055ab2762a1bd in as_list_list_del (h=0x55ab27897680 <as_exclude_list_orphan>, item=0x55ab28204e20) at ./bgpd/bgp_aspath.h:77
> FRRouting#4  0x000055ab2762d1a8 in as_exclude_remove_orphan (ase=0x55ab28204e20) at bgpd/bgp_aspath.c:1574
> FRRouting#5  0x000055ab27550538 in route_aspath_exclude_free (rule=0x55ab28204e20) at bgpd/bgp_routemap.c:2366
> FRRouting#6  0x00007fb39591f00c in route_map_rule_delete (list=0x55ab28203498, rule=0x55ab28204170) at lib/routemap.c:1357
> FRRouting#7  0x00007fb39591f87c in route_map_add_set (index=0x55ab28203460, set_name=0x55ab276ad2aa "as-path exclude", set_arg=0x55ab281e4f70 "as-path-access-list test") at lib/routemap.c:1674
> FRRouting#8  0x00007fb39591d3f3 in generic_set_add (index=0x55ab28203460, command=0x55ab276ad2aa "as-path exclude", arg=0x55ab281e4f70 "as-path-access-list test", errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/routemap.c:533
> FRRouting#9  0x000055ab2755e78e in lib_route_map_entry_set_action_rmap_set_action_exclude_as_path_modify (args=0x7ffd122db290) at bgpd/bgp_routemap_nb_config.c:2427
> FRRouting#10 0x00007fb3958fe417 in nb_callback_modify (context=0x55ab28205aa0, nb_node=0x55ab27cb31e0, event=NB_EV_APPLY, dnode=0x55ab28202690, resource=0x55ab27c32148, errmsg=0x7ffd122db870 "",
>     errmsg_len=8192) at lib/northbound.c:1538
> FRRouting#11 0x00007fb3958ff0ab in nb_callback_configuration (context=0x55ab28205aa0, event=NB_EV_APPLY, change=0x55ab27c32110, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:1888
> FRRouting#12 0x00007fb3958ff5e4 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x55ab28205aa0, errmsg=0x7ffd122db870 "", errmsg_len=8192) at lib/northbound.c:2016
> FRRouting#13 0x00007fb3958fddba in nb_candidate_commit_apply (transaction=0x55ab28205aa0, save_transaction=true, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1356
> FRRouting#14 0x00007fb3958fdef0 in nb_candidate_commit (context=..., candidate=0x55ab27c2c9a0, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffd122db870 "", errmsg_len=8192)
>     at lib/northbound.c:1389
> FRRouting#15 0x00007fb3959045ba in nb_cli_classic_commit (vty=0x55ab281f6680) at lib/northbound_cli.c:57
> FRRouting#16 0x00007fb395904b5a in nb_cli_apply_changes_internal (vty=0x55ab281f6680, xpath_base=0x7ffd122dfd10 "/frr-route-map:lib/route-map[name='routemaptest']/entry[sequence='1']",
>     clear_pending=false) at lib/northbound_cli.c:184
> FRRouting#17 0x00007fb395904ebf in nb_cli_apply_changes (vty=0x55ab281f6680, xpath_base_fmt=0x0) at lib/northbound_cli.c:240
> --Type <RET> for more, q to quit, c to continue without paging--
> FRRouting#18 0x000055ab27557d2e in set_aspath_exclude_access_list_magic (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80,
>     as_path_filter_name=0x55ab28202040 "test") at bgpd/bgp_routemap.c:6397
> FRRouting#19 0x000055ab2754bdea in set_aspath_exclude_access_list (self=0x55ab2775c300 <set_aspath_exclude_access_list_cmd>, vty=0x55ab281f6680, argc=5, argv=0x55ab28204c80)
>     at ./bgpd/bgp_routemap_clippy.c:856
> FRRouting#20 0x00007fb39589435d in cmd_execute_command_real (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, up_level=0) at lib/command.c:1003
> FRRouting#21 0x00007fb3958944be in cmd_execute_command (vline=0x55ab281e61f0, vty=0x55ab281f6680, cmd=0x0, vtysh=0) at lib/command.c:1062
> FRRouting#22 0x00007fb395894a0c in cmd_execute (vty=0x55ab281f6680, cmd=0x55ab28200f20 "set as-path exclude as-path-access-list test", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#23 0x00007fb39595242c in vty_command (vty=0x55ab281f6680, buf=0x55ab28200f20 "set as-path exclude as-path-access-list test") at lib/vty.c:625
> FRRouting#24 0x00007fb3959541c5 in vty_execute (vty=0x55ab281f6680) at lib/vty.c:1388
> FRRouting#25 0x00007fb3959563db in vtysh_read (thread=0x7ffd122e2bb0) at lib/vty.c:2400
> FRRouting#26 0x00007fb39594b785 in event_call (thread=0x7ffd122e2bb0) at lib/event.c:1996
> FRRouting#27 0x00007fb3958d1365 in frr_run (master=0x55ab27b56d70) at lib/libfrr.c:1231
> FRRouting#28 0x000055ab2747f1cc in main (argc=3, argv=0x7ffd122e2e08) at bgpd/bgp_main.c:555

Fixes: 094dcc3 ("bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
enkechen-panw pushed a commit to enkechen-panw/frr that referenced this pull request Sep 14, 2024
The following causes a isisd crash.

> # cat config
> affinity-map green bit-position 0
> router isis 1
>  flex-algo 129
>   affinity exclude-any green
> # vtysh -f config

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#1  0x00007f650cd32756 in core_handler (signo=6, siginfo=0x7ffc56f93070, context=0x7ffc56f92f40) at lib/sigevent.c:258
> FRRouting#2  <signal handler called>
> FRRouting#3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#4  0x00007f650c91c537 in __GI_abort () at abort.c:79
> FRRouting#5  0x00007f650cd007c9 in nb_running_get_entry_worker (dnode=0x0, xpath=0x0, abort_if_not_found=true, rec_search=true) at lib/northbound.c:2531
> FRRouting#6  0x00007f650cd007f9 in nb_running_get_entry (dnode=0x55d9ad406e00, xpath=0x0, abort_if_not_found=true) at lib/northbound.c:2537
> FRRouting#7  0x000055d9ab302248 in isis_instance_flex_algo_affinity_set (args=0x7ffc56f947a0, type=2) at isisd/isis_nb_config.c:2998
> FRRouting#8  0x000055d9ab3027c0 in isis_instance_flex_algo_affinity_exclude_any_create (args=0x7ffc56f947a0) at isisd/isis_nb_config.c:3155
> FRRouting#9  0x00007f650ccfe284 in nb_callback_create (context=0x7ffc56f94d20, nb_node=0x55d9ad28b540, event=NB_EV_VALIDATE, dnode=0x55d9ad406e00, resource=0x0, errmsg=0x7ffc56f94de0 "",
>     errmsg_len=8192) at lib/northbound.c:1487
> FRRouting#10 0x00007f650ccff067 in nb_callback_configuration (context=0x7ffc56f94d20, event=NB_EV_VALIDATE, change=0x55d9ad406d40, errmsg=0x7ffc56f94de0 "", errmsg_len=8192) at lib/northbound.c:1884
> FRRouting#11 0x00007f650ccfda31 in nb_candidate_validate_code (context=0x7ffc56f94d20, candidate=0x55d9ad20d710, changes=0x7ffc56f94d38, errmsg=0x7ffc56f94de0 "", errmsg_len=8192)
>     at lib/northbound.c:1246
> FRRouting#12 0x00007f650ccfdc67 in nb_candidate_commit_prepare (context=..., candidate=0x55d9ad20d710, comment=0x0, transaction=0x7ffc56f94da0, skip_validate=false, ignore_zero_change=false,
>     errmsg=0x7ffc56f94de0 "", errmsg_len=8192) at lib/northbound.c:1317
> FRRouting#13 0x00007f650ccfdec4 in nb_candidate_commit (context=..., candidate=0x55d9ad20d710, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffc56f94de0 "", errmsg_len=8192)
>     at lib/northbound.c:1381
> FRRouting#14 0x00007f650cd045ba in nb_cli_classic_commit (vty=0x55d9ad3f7490) at lib/northbound_cli.c:57
> FRRouting#15 0x00007f650cd04749 in nb_cli_pending_commit_check (vty=0x55d9ad3f7490) at lib/northbound_cli.c:96
> FRRouting#16 0x00007f650cc94340 in cmd_execute_command_real (vline=0x55d9ad3eea10, vty=0x55d9ad3f7490, cmd=0x0, up_level=0) at lib/command.c:1000
> FRRouting#17 0x00007f650cc94599 in cmd_execute_command (vline=0x55d9ad3eea10, vty=0x55d9ad3f7490, cmd=0x0, vtysh=0) at lib/command.c:1080
> FRRouting#18 0x00007f650cc94a0c in cmd_execute (vty=0x55d9ad3f7490, cmd=0x55d9ad401d30 "XFRR_end_configuration", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#19 0x00007f650cd523a4 in vty_command (vty=0x55d9ad3f7490, buf=0x55d9ad401d30 "XFRR_end_configuration") at lib/vty.c:625
> FRRouting#20 0x00007f650cd5413d in vty_execute (vty=0x55d9ad3f7490) at lib/vty.c:1388
> FRRouting#21 0x00007f650cd56353 in vtysh_read (thread=0x7ffc56f99370) at lib/vty.c:2400
> FRRouting#22 0x00007f650cd4b6fd in event_call (thread=0x7ffc56f99370) at lib/event.c:1996
> FRRouting#23 0x00007f650ccd1365 in frr_run (master=0x55d9ad103cf0) at lib/libfrr.c:1231
> FRRouting#24 0x000055d9ab29036e in main (argc=2, argv=0x7ffc56f99598, envp=0x7ffc56f995b0) at isisd/isis_main.c:354

Configuring the same in vtysh configure interactive mode works properly.
When using "vtysh -f", the northbound compatible configuration is
committed together whereas, in interactive mode, it committed line by
line. In the first situation, in validation state nb_running_get_entry()
fails because the area not yet in running.

Do not use nb_running_get_entry() northbound validation state.

Fixes: 893882e ("isisd: add isis flex-algo configuration backend")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
enkechen-panw pushed a commit to enkechen-panw/frr that referenced this pull request Sep 14, 2024
Level 2 adjacency list is not supposed to be always set.

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#1  0x00007f9f0353274f in core_handler (signo=6, siginfo=0x7ffe95260770, context=0x7ffe95260640) at lib/sigevent.c:258
> FRRouting#2  <signal handler called>
> FRRouting#3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> FRRouting#4  0x00007f9f0324e537 in __GI_abort () at abort.c:79
> FRRouting#5  0x00007f9f035744ea in _zlog_assert_failed (xref=0x7f9f0362c6c0 <_xref.15>, extra=0x0) at lib/zlog.c:789
> FRRouting#6  0x00007f9f034d25ee in listnode_head (list=0x0) at lib/linklist.c:316
> FRRouting#7  0x000055cd65aaa481 in lib_interface_state_isis_adjacencies_adjacency_get_next (args=0x7ffe95261730) at isisd/isis_nb_state.c:101
> FRRouting#8  0x00007f9f034feadd in nb_callback_get_next (nb_node=0x55cd673c0190, parent_list_entry=0x55cd67570d30, list_entry=0x55cd6758f8a0) at lib/northbound.c:1748
> FRRouting#9  0x00007f9f0350bf07 in __walk (ys=0x55cd675782b0, is_resume=false) at lib/northbound_oper.c:1264
> FRRouting#10 0x00007f9f0350deaa in nb_op_walk_start (ys=0x55cd675782b0) at lib/northbound_oper.c:1741
> FRRouting#11 0x00007f9f0350e079 in nb_oper_iterate_legacy (xpath=0x55cd67595c60 "/frr-interface:lib", translator=0x0, flags=0, cb=0x0, cb_arg=0x0, tree=0x7ffe952621b0) at lib/northbound_oper.c:1803
> FRRouting#12 0x00007f9f03507661 in show_yang_operational_data_magic (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0,
>     xpath=0x55cd67595c60 "/frr-interface:lib", json=0x0, xml=0x0, translator_family=0x0, with_config=0x0) at lib/northbound_cli.c:1576
> FRRouting#13 0x00007f9f035037f0 in show_yang_operational_data (self=0x7f9f03634a80 <show_yang_operational_data_cmd>, vty=0x55cd675a61f0, argc=4, argv=0x55cd6758eab0)
>     at ./lib/northbound_cli_clippy.c:906
> FRRouting#14 0x00007f9f0349435d in cmd_execute_command_real (vline=0x55cd6758e490, vty=0x55cd675a61f0, cmd=0x0, up_level=0) at lib/command.c:1003
> FRRouting#15 0x00007f9f03494477 in cmd_execute_command (vline=0x55cd67585340, vty=0x55cd675a61f0, cmd=0x0, vtysh=0) at lib/command.c:1053
> FRRouting#16 0x00007f9f03494a0c in cmd_execute (vty=0x55cd675a61f0, cmd=0x55cd67579040 "do show yang operational-data /frr-interface:lib", matched=0x0, vtysh=0) at lib/command.c:1228
> FRRouting#17 0x00007f9f0355239d in vty_command (vty=0x55cd675a61f0, buf=0x55cd67579040 "do show yang operational-data /frr-interface:lib") at lib/vty.c:625
> FRRouting#18 0x00007f9f03554136 in vty_execute (vty=0x55cd675a61f0) at lib/vty.c:1388
> FRRouting#19 0x00007f9f0355634c in vtysh_read (thread=0x7ffe952647a0) at lib/vty.c:2400
> FRRouting#20 0x00007f9f0354b6f6 in event_call (thread=0x7ffe952647a0) at lib/event.c:1996
> FRRouting#21 0x00007f9f034d1365 in frr_run (master=0x55cd67204da0) at lib/libfrr.c:1231
> FRRouting#22 0x000055cd65a3236e in main (argc=7, argv=0x7ffe952649c8, envp=0x7ffe95264a08) at isisd/isis_main.c:354

Fixes: 2a1c520 ("isisd: split northbound callbacks into multiple files")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
jyuan-panw added a commit to jyuan-panw/frr that referenced this pull request Sep 18, 2024
A crash was observed in 8.4.4 zebra when FRR processes were being shutting down by
automated script:

  Thread 1 (LWP 16051):
  #0  0x00007f63f449bb8f in raise () from /lib64/libpthread.so.0
  FRRouting#1  0x00007f63f5c68300 in core_handler (signo=11, siginfo=0x7ffec6322cb0, context=<optimized out>) at lib/sigevent.c:261
  FRRouting#2  <signal handler called>
  FRRouting#3  zebra_router_get_table (zvrf=zvrf@entry=0x0, tableid=tableid@entry=254, afi=afi@entry=AFI_IP, safi=safi@entry=SAFI_UNICAST) at /usr/include/bits/string_fortified.h:71
  FRRouting#4  0x0000560d74192e6d in zebra_vrf_get_table_with_table_id (afi=AFI_IP, safi=SAFI_UNICAST, vrf_id=<optimized out>, table_id=254) at zebra/zebra_vrf.c:335
  FRRouting#5  0x0000560d74186d20 in process_subq_early_route_add (ere=<optimized out>) at zebra/zebra_rib.c:2649
  FRRouting#6  process_subq_early_route (lnode=0x560d7783c5f0) at zebra/zebra_rib.c:3127
  FRRouting#7  process_subq (qindex=META_QUEUE_EARLY_ROUTE, subq=0x560d75cb1e40) at zebra/zebra_rib.c:3150
  FRRouting#8  meta_queue_process (dummy=<optimized out>, data=0x560d75cba680) at zebra/zebra_rib.c:3202
  FRRouting#9  0x00007f63f5c84550 in work_queue_run (thread=0x7ffec63233d0) at lib/workqueue.c:285
  FRRouting#10 0x00007f63f5c7a4c1 in thread_call (thread=thread@entry=0x7ffec63233d0) at lib/thread.c:2008
  FRRouting#11 0x00007f63f5c32088 in frr_run (master=0x560d75acbf50) at lib/libfrr.c:1216
  FRRouting#12 0x0000560d7411c8f7 in main (argc=<optimized out>, argv=0x7ffec63237a8) at zebra/main.c:499

Below is analysis for the sequence of events which led to zebra crash:

 - configs including VRF configs were deleted in zebra
 - Some route messages for the deleted VRF were still in the zebra metaq waiting
   to be processed
 - when the route message was dequeued for processing, the VRF was already deleted
 - lookup of zvrf failed for the route vrf_id in route-entry, but the NULL return
   was not checked, resulted in SIGSEGV crash when it was dereferenced later

Signed-off-by: Jenny Yuan <jyuan@paloaltonetworks.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 20, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 20, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 24, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 24, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 24, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
riw777 pushed a commit that referenced this pull request Sep 24, 2024
```
==5445==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x7ff4c6bedb19 bp 0x7ffc95f2e400 sp 0x7ffc95f2e3c0 T0)
==5445==The signal is caused by a READ memory access.
==5445==Hint: address points to the zero page.
    #0 0x7ff4c6bedb19 in hash_iterate lib/hash.c:246
    #1 0x5618f41f5f59 in bgp_evpn_nh_finish bgpd/bgp_evpn_mh.c:4663
    #2 0x5618f41dcbe8 in bgp_evpn_vrf_delete bgpd/bgp_evpn.c:7336
    #3 0x5618f43bdd35 in bgp_delete bgpd/bgpd.c:4098
    #4 0x5618f417ef6e in bgp_exit bgpd/bgp_main.c:206
    #5 0x5618f417ef6e in sigint bgpd/bgp_main.c:164
    #6 0x7ff4c6cac6c4 in frr_sigevent_process lib/sigevent.c:117
    #7 0x7ff4c6cd8258 in event_fetch lib/event.c:1767
    #8 0x7ff4c6c0dcbc in frr_run lib/libfrr.c:1230
    #9 0x5618f418080d in main bgpd/bgp_main.c:555
    #10 0x7ff4c670c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #11 0x7ff4c670c304 in __libc_start_main_impl ../csu/libc-start.c:360
    #12 0x5618f417ea20 in _start (/usr/lib/frr/bgpd+0x2e4a20)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV lib/hash.c:246 in hash_iterate
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 25, 2024
Fix a heap-after-free that causes zebra to crash even without
address-sanitizer. To reproduce:

> echo "100 my_table" | tee -a /etc/iproute2/rt_tables
> ip route add blackhole default table 100
> ip route show table 100
> ip l add red type vrf table 100
> ip l del red
> ip route del blackhole default table 100

Zebra manages routing tables for all existing Linux RT tables,
regardless of whether they are assigned to a VRF interface. When a table
is not assigned to any VRF, zebra arbitrarily assigns it to the default
VRF, even though this is not strictly accurate (the code expects this
behavior).

When an RT table is created after a VRF, zebra correctly assigns the
table to the VRF. However, if a VRF interface is assigned to an existing
RT table, zebra does not update the table owner, which remains as the
default VRF. As a result, existing routing entries remain under the
default VRF, while new entries are correctly assigned to the VRF. The
VRF mismatch is unexpected in the code and creates crashes and memory
related issues.

Furthermore, Linux does not automatically delete RT tables when they are
unassigned from a VRF. It is incorrect to delete these tables from zebra.

Instead, at VRF disabling, do not release the table but reassign it to
the default VRF. At VRF enabling, change the table owner back to the
appropriate VRF.

> ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
> READ of size 1 at 0x606000154f54 thread T0
>     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
>     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
>     #2 0x7fa32474d783 in route_node_get lib/table.c:283
>     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
>     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
>     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
>     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
>     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
>
> 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
> freed by thread T0 here:
>     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
>     #1 0x7fa324668d8f in qfree lib/memory.c:130
>     #2 0x7fa32474c421 in route_table_free lib/table.c:126
>     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
>     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
>     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
>     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
>     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
>     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
>     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
>     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
>     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
>     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
>     #3 0x7fa32474e73c in route_table_init lib/table.c:512
>     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
>     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
>     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
>     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
>     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
>     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
>     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
>     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
>     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
>     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
>     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
>     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308

Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
louis-6wind added a commit to louis-6wind/frr that referenced this pull request Sep 25, 2024
Fix heap-after-free seen with bgp_vrf_dynamic_route_leak /
test_bgp_vrf_dynamic_route_leak_topo1 topotest.

> =================================================================
> ==1899==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000acba4 at pc 0x559fe9aee213 bp 0x7fffef3d3ef0 sp 0x7fffef3d3ee8
> READ of size 4 at 0x6160000acba4 thread T0
>     #0 0x559fe9aee212 in ctx_info_from_zns zebra/zebra_dplane.c:3331
>     #1 0x559fe9aee212 in dplane_ctx_ns_init zebra/zebra_dplane.c:3347
>     #2 0x559fe9af50a2 in dplane_ctx_nexthop_init zebra/zebra_dplane.c:3696
>     #3 0x559fe9afadd1 in dplane_nexthop_update_internal zebra/zebra_dplane.c:4503
>     FRRouting#4 0x559fe9afb0ff in dplane_nexthop_delete zebra/zebra_dplane.c:4730
>     FRRouting#5 0x559fe9b6fddf in zebra_nhg_uninstall_kernel zebra/zebra_nhg.c:3278
>     FRRouting#6 0x559fe9b700bf in zebra_nhg_decrement_ref zebra/zebra_nhg.c:1768
>     FRRouting#7 0x559fe9b8a5d3 in route_entry_update_nhe zebra/zebra_rib.c:457
>     FRRouting#8 0x559fe9b932f7 in rib_re_nhg_free zebra/zebra_rib.c:2691
>     FRRouting#9 0x559fe9b932f7 in rib_unlink zebra/zebra_rib.c:4088
>     FRRouting#10 0x559fe9b93485 in zebra_rtable_node_cleanup zebra/zebra_rib.c:958
>     FRRouting#11 0x7fd4040c6672 in route_node_free lib/table.c:75
>     FRRouting#12 0x7fd4040c7378 in route_table_free lib/table.c:111
>     FRRouting#13 0x7fd4040c7378 in route_table_finish lib/table.c:46
>     FRRouting#14 0x559fe9b9cd50 in zebra_router_free_table zebra/zebra_router.c:191
>     FRRouting#15 0x559fe9b9e3fe in zebra_router_terminate zebra/zebra_router.c:244
>     FRRouting#16 0x559fe9aa3c40 in zebra_finalize zebra/main.c:244
>     FRRouting#17 0x7fd4040da2c1 in event_call lib/event.c:1996
>     FRRouting#18 0x7fd40400dcc6 in frr_run lib/libfrr.c:1237
>     FRRouting#19 0x559fe9aa435e in main zebra/main.c:526
>     FRRouting#20 0x7fd403b0c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>     FRRouting#21 0x7fd403b0c304 in __libc_start_main_impl ../csu/libc-start.c:360
>     FRRouting#22 0x559fe9a777a0 in _start (/usr/lib/frr/zebra+0x1a47a0)
>
> 0x6160000acba4 is located 36 bytes inside of 584-byte region [0x6160000acb80,0x6160000acdc8)
> freed by thread T0 here:
>     #0 0x7fd4044b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
>     #1 0x7fd40402d060 in qfree lib/memory.c:131
>     #2 0x559fe9b73aaa in zebra_ns_delete zebra/zebra_ns.c:74
>     #3 0x559fe9b7415a in zebra_ns_final_shutdown zebra/zebra_ns.c:192
>     FRRouting#4 0x7fd40404d45e in ns_walk_func lib/netns_linux.c:372
>     FRRouting#5 0x559fe9aa3c36 in zebra_finalize zebra/main.c:241
>     FRRouting#6 0x7fd4040da2c1 in event_call lib/event.c:1996
>     FRRouting#7 0x7fd40400dcc6 in frr_run lib/libfrr.c:1237
>     FRRouting#8 0x559fe9aa435e in main zebra/main.c:526
>     FRRouting#9 0x7fd403b0c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> previously allocated by thread T0 here:
>     #0 0x7fd4044b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
>     #1 0x7fd40402c76d in qcalloc lib/memory.c:106
>     #2 0x559fe9b73b9b in zebra_ns_new zebra/zebra_ns.c:55
>     #3 0x559fe9b74283 in zebra_ns_init zebra/zebra_ns.c:221
>     FRRouting#4 0x559fe9aa4159 in main zebra/main.c:440
>     FRRouting#5 0x7fd403b0c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Jafaral added a commit to Jafaral/frr that referenced this pull request Sep 25, 2024
```
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000aecf0 at pc 0x5555557ecdb9 bp 0x7fffffffe350 sp 0x7fffffffe340
READ of size 4 at 0x6160000aecf0 thread T0
    #0 0x5555557ecdb8 in igmp_source_delete pimd/pim_igmpv3.c:340
    FRRouting#1 0x5555557ed475 in igmp_source_delete_expired pimd/pim_igmpv3.c:405
    FRRouting#2 0x5555557de574 in igmp_group_timer pimd/pim_igmp.c:1346
    FRRouting#3 0x7ffff7275421 in event_call lib/event.c:1996
    FRRouting#4 0x7ffff7140797 in frr_run lib/libfrr.c:1237
    FRRouting#5 0x5555557f5840 in main pimd/pim_main.c:166
    FRRouting#6 0x7ffff6a54082 in __libc_start_main ../csu/libc-start.c:308
    FRRouting#7 0x555555686eed in _start (/usr/lib/frr/pimd+0x132eed)

0x6160000aecf0 is located 112 bytes inside of 600-byte region [0x6160000aec80,0x6160000aeed8)
freed by thread T0 here:
    #0 0x7ffff767b40f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
    FRRouting#1 0x7ffff716ed34 in qfree lib/memory.c:131
    FRRouting#2 0x5555557169ae in pim_channel_oil_free pimd/pim_oil.c:84
    FRRouting#3 0x555555717981 in pim_channel_oil_del pimd/pim_oil.c:199
    FRRouting#4 0x55555573c42c in tib_sg_gm_prune pimd/pim_tib.c:196
    FRRouting#5 0x5555557d6d04 in igmp_source_forward_stop pimd/pim_igmp.c:229
    FRRouting#6 0x5555557d5855 in igmp_anysource_forward_stop pimd/pim_igmp.c:61
    FRRouting#7 0x5555557de539 in igmp_group_timer pimd/pim_igmp.c:1344
    FRRouting#8 0x7ffff7275421 in event_call lib/event.c:1996
    FRRouting#9 0x7ffff7140797 in frr_run lib/libfrr.c:1237
    FRRouting#10 0x5555557f5840 in main pimd/pim_main.c:166
    FRRouting#11 0x7ffff6a54082 in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
    #0 0x7ffff767ba06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    FRRouting#1 0x7ffff716ebe1 in qcalloc lib/memory.c:106
    FRRouting#2 0x555555716eb7 in pim_channel_oil_add pimd/pim_oil.c:133
    FRRouting#3 0x55555573b2b9 in tib_sg_oil_setup pimd/pim_tib.c:30
    FRRouting#4 0x55555573bdd3 in tib_sg_gm_join pimd/pim_tib.c:119
    FRRouting#5 0x5555557d6788 in igmp_source_forward_start pimd/pim_igmp.c:193
    FRRouting#6 0x5555557d5771 in igmp_anysource_forward_start pimd/pim_igmp.c:51
    FRRouting#7 0x5555557ecaa0 in group_exclude_fwd_anysrc_ifempty pimd/pim_igmpv3.c:310
    FRRouting#8 0x5555557ef937 in toex_incl pimd/pim_igmpv3.c:839
    FRRouting#9 0x5555557f00a2 in igmpv3_report_toex pimd/pim_igmpv3.c:938
    FRRouting#10 0x5555557f543d in igmp_v3_recv_report pimd/pim_igmpv3.c:2000
    FRRouting#11 0x5555557da2b4 in pim_igmp_packet pimd/pim_igmp.c:787
    FRRouting#12 0x5555556ee46a in process_igmp_packet pimd/pim_mroute.c:763
    FRRouting#13 0x5555556ee5f3 in pim_mroute_msg pimd/pim_mroute.c:787
    FRRouting#14 0x5555556eef58 in mroute_read pimd/pim_mroute.c:877
    FRRouting#15 0x7ffff7275421 in event_call lib/event.c:1996
    FRRouting#16 0x7ffff7140797 in frr_run lib/libfrr.c:1237
    FRRouting#17 0x5555557f5840 in main pimd/pim_main.c:166
    FRRouting#18 0x7ffff6a54082 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: heap-use-after-free pimd/pim_igmpv3.c:340 in igmp_source_delete
Shadow bytes around the buggy address:
  0x0c2c8000dd40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000dd50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000dd60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000dd70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000dd80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2c8000dd90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd
  0x0c2c8000dda0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000ddb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000ddc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8000ddd0: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c2c8000dde0: 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
```

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants