pathd: fix heap use after deletion #28

GalaxyGorilla · 2020-06-29T15:46:45Z

Signed-off-by: GalaxyGorilla sascha@netdef.org

Signed-off-by: GalaxyGorilla <sascha@netdef.org>

GalaxyGorilla · 2020-06-29T15:48:35Z

This is supposed to fix

    #5 0x495007 in nb_candidate_commit lib/northbound.c:756
    #6 0x49ad84 in nb_cli_apply_changes lib/northbound_cli.c:168
    #7 0x41c45c in no_te_path_segment_list_segment_magic pathd/path_cli.c:288
    #8 0x41c7e7 in no_te_path_segment_list_segment pathd/path_cli_clippy.c:313
    #9 0x42be38 in cmd_execute_command_real lib/command.c:907
    #10 0x430d8b in cmd_execute_command lib/command.c:966
    #11 0x43106b in cmd_execute lib/command.c:1120
    #12 0x4f8e3b in vty_command lib/vty.c:526
    #13 0x4f93f8 in vty_execute lib/vty.c:1293
    #14 0x4feeab in vtysh_read lib/vty.c:2126
    #15 0x4ed480 in thread_call lib/thread.c:1549
    #16 0x47aac6 in frr_run lib/libfrr.c:1098
    #17 0x41b58a in main pathd/path_main.c:150
    #18 0x7fd81974282f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #19 0x41b258 in _start (/usr/lib/frr/pathd+0x41b258)

0x60b00002b610 is located 32 bytes inside of 104-byte region [0x60b00002b5f0,0x60b00002b658)
freed by thread T0 here:
    #0 0x7fd81a8f72ca in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x982ca)
    #1 0x4830fe in qfree lib/memory.c:129
    #2 0x425201 in srte_segment_entry_del pathd/pathd.c:137
    #3 0x42113f in pathd_te_segment_list_segment_destroy pathd/path_nb_config.c:135
    #4 0x493657 in nb_callback_destroy lib/northbound.c:949
    #5 0x493657 in nb_callback_configuration lib/northbound.c:1171
    #6 0x49423b in nb_transaction_process lib/northbound.c:1288
    #7 0x494974 in nb_candidate_commit_apply lib/northbound.c:724
    #8 0x495007 in nb_candidate_commit lib/northbound.c:756
    #9 0x49ad84 in nb_cli_apply_changes lib/northbound_cli.c:168
    #10 0x41c45c in no_te_path_segment_list_segment_magic pathd/path_cli.c:288
    #11 0x41c7e7 in no_te_path_segment_list_segment pathd/path_cli_clippy.c:313
    #12 0x42be38 in cmd_execute_command_real lib/command.c:907
    #13 0x430d8b in cmd_execute_command lib/command.c:966
    #14 0x43106b in cmd_execute lib/command.c:1120
    #15 0x4f8e3b in vty_command lib/vty.c:526
    #16 0x4f93f8 in vty_execute lib/vty.c:1293
    #17 0x4feeab in vtysh_read lib/vty.c:2126
    #18 0x4ed480 in thread_call lib/thread.c:1549
    #19 0x47aac6 in frr_run lib/libfrr.c:1098
    #20 0x41b58a in main pathd/path_main.c:150
    #21 0x7fd81974282f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

previously allocated by thread T0 here:
    #0 0x7fd81a8f779a in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9879a)
    #1 0x482958 in qcalloc lib/memory.c:110
    #2 0x425151 in srte_segment_entry_add pathd/pathd.c:125
    #3 0x42107d in pathd_te_segment_list_segment_create pathd/path_nb_config.c:120
    #4 0x4933d0 in nb_callback_create lib/northbound.c:852
    #5 0x4933d0 in nb_callback_configuration lib/northbound.c:1163
    #6 0x49423b in nb_transaction_process lib/northbound.c:1288
    #7 0x494974 in nb_candidate_commit_apply lib/northbound.c:724
    #8 0x495007 in nb_candidate_commit lib/northbound.c:756
    #9 0x49ad84 in nb_cli_apply_changes lib/northbound_cli.c:168
    #10 0x41caec in te_path_segment_list_segment_magic pathd/path_cli.c:274
    #11 0x41eb97 in te_path_segment_list_segment pathd/path_cli_clippy.c:260
    #12 0x42be38 in cmd_execute_command_real lib/command.c:907
    #13 0x430d8b in cmd_execute_command lib/command.c:966
    #14 0x43106b in cmd_execute lib/command.c:1120
    #15 0x4f8e3b in vty_command lib/vty.c:526
    #16 0x4f93f8 in vty_execute lib/vty.c:1293
    #17 0x4feeab in vtysh_read lib/vty.c:2126
    #18 0x4ed480 in thread_call lib/thread.c:1549
    #19 0x47aac6 in frr_run lib/libfrr.c:1098
    #20 0x41b58a in main pathd/path_main.c:150
    #21 0x7fd81974282f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-use-after-free pathd/path_nb_config.c:136 pathd_te_segment_list_segment_destroy
Shadow bytes around the buggy address:
  0x0c167fffd670: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c167fffd680: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0c167fffd690: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c167fffd6a0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c167fffd6b0: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fd fd
=>0x0c167fffd6c0: fd fd[fd]fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c167fffd6d0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c167fffd6e0: fd fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd
  0x0c167fffd6f0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c167fffd700: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
  0x0c167fffd710: fa fa fa fa fa fa fd fd fd fd fd fd fd fd fd fd
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
rt1: Daemon pathd killed by AddressSanitizer2020-06-29 13:24:23,868 ERROR: assert failed at "test_isis_sr_te_topo1/test_memory_leak":
rt1: Daemon pathd killed by AddressSanitizer
Address Sanitizer triggered - Test failed

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>

… the neighbor status remains UP Test Scenario: RouterA and RouterB are in the same routing domain and have configured a P2P link. RouterA is configured with "is-type level-1" while RouterB is configured with "is-type level-1-2". They establish a level-1 UP neighborship. In this scenario, we expect that when RouterB's configuration is switched to "is-type level-2-only", the neighborship status on both RouterA and RouterB would be non-UP. However, RouterB still shows the neighbor as UP. Upon receiving a P2P Hello packet, the function "process_p2p_hello" is invoked. According to the ISO/IEC 10589 protocol specification, section 8.2.5.2 a) and tables 5 and 7, if the "iih->circ_type" of the neighbor's hello packet does not match one's own "circuit->is_type," we may choose to take no action. When establishing a neighborship for the first time, the neighbor's status can remain in the "Initializing" state. However, if the neighborship has already been established and one's own "circuit->is_type" changes, the neighbor's UP status cannot be reset. Therefore, when processing P2P Hello packets, we should be cognizant of changes in our own link adjacency type. Topotest has identified a core issue during testing. (gdb) bt "#0 0xb7efe579 in __kernel_vsyscall () \#1 0xb79f62f7 in ?? () \#2 0xbf981dd0 in ?? () \#3 <signal handler called> \#4 0xb79f7722 in ?? () \#5 0xb7ed8634 in _DYNAMIC () from /home/z15467/isis_core/usr/lib/i386-linux-gnu/frr/libfrr.so.0.0.0 \#6 0x0001003c in ?? () \#7 0x00010000 in ?? () \#8 0xb7df3322 in _frr_mtx_lock (mutex=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/frr_pthread.h:255 \#9 event_timer_remain_msec (thread=0x10000) at ../lib/event.c:734 \#10 event_timer_remain_msec (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:727 \#11 0x004fb4aa in _send_hello_sched (circuit=<optimized out>, threadp=0x2189de0, level=1, delay=<optimized out>) at ../isisd/isis_pdu.c:2116 \#12 0x004e8dbc in isis_circuit_up (circuit=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_circuit.c:734 \#13 0x004ea8f7 in isis_csm_state_change (event=<optimized out>, circuit=<optimized out>, arg=<optimized out>) at ../isisd/isis_csm.c:98 \#14 0x004ea23f in isis_circuit_circ_type_set (circuit=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, circ_type=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_circuit.c:1578 \#15 0x0053aefa in lib_interface_isis_network_type_modify (args=<optimized out>) at ../isisd/isis_nb_config.c:4190 \#16 0xb7dbcc8d in nb_callback_modify (errmsg_len=8192, errmsg=0xbf982afc "", resource=0x2186220, dnode=<optimized out>, event=NB_EV_APPLY, nb_node=0x1fafe70, context=<optimized out>) at ../lib/northbound.c:1550 \#17 nb_callback_configuration (context=<optimized out>, event=NB_EV_APPLY, change=<optimized out>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1900 \#18 0xb7dbd646 in nb_transaction_process (errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, event=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:2028 \#19 nb_candidate_commit_apply (transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, save_transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction_id=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1368 \#20 0xb7dbdd68 in nb_candidate_commit (context=..., candidate=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, save_transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, comment=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction_id=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1401 \#21 0xb7dc0cff in nb_cli_classic_commit (vty=vty@entry=0x21d6940) at ../lib/northbound_cli.c:57 \#22 0xb7dc0f46 in nb_cli_apply_changes_internal (vty=vty@entry=0x21d6940, xpath_base=xpath_base@entry=0xbf986b7c "/frr-interface:lib/interface[name='r5-eth0']", clear_pending=clear_pending@entry=false) at ../lib/northbound_cli.c:184 \#23 0xb7dc130b in nb_cli_apply_changes (vty=<optimized out>, xpath_base_fmt=<optimized out>) at ../lib/northbound_cli.c:240 \#24 0x00542c1d in isis_network_magic (self=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argc=<optimized out>, argv=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, no=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_cli.c:3101 \#25 isis_network (self=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argc=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argv=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ./isisd/isis_cli_clippy.c:5499 \#26 0xb7d6d8f1 in cmd_execute_command_real (vline=vline@entry=0x219afa0, vty=vty@entry=0x21d6940, cmd=cmd@entry=0x0, up_level=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1003 \#27 0xb7d6d9e0 in cmd_execute_command (vline=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, cmd=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vtysh=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1061 \#28 0xb7d6dc60 in cmd_execute (vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, cmd=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, matched=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vtysh=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1228 \#29 0xb7dfb58a in vty_command (vty=vty@entry=0x21d6940, buf=0x21e0ff0 ' ' <repeats 12 times>, "isis network point-to-point") at ../lib/vty.c:625 \#30 0xb7dfc560 in vty_execute (vty=vty@entry=0x21d6940) at ../lib/vty.c:1388 \#31 0xb7dfdc8d in vtysh_read (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/vty.c:2400 \#32 0xb7df4d47 in event_call (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:2019 \#33 0xb7d9a831 in frr_run (master=<optimized out>) at ../lib/libfrr.c:1232 \#34 0x004e4758 in main (argc=7, argv=0xbf989a24, envp=0xbf989a44) at ../isisd/isis_main.c:354 (gdb) f 9 \#9 event_timer_remain_msec (thread=0x10000) at ../lib/event.c:734 734 ../lib/event.c: No such file or directory. (gdb) p pthread No symbol "pthread" in current context. (gdb) p thread $1 = (struct event *) 0x10000 When LAN links and P2P links share the` circuit->u` of a neighbor, if one link is no longer in use and the union is not cleared, the other link is unable to pass the non-empty check, resulting in accessing an invalid pointer. Unfortunately, for non-DIS devices in LAN links, `circuit->u.bc.run_dr_elect[x]` is essentially always 1, but in `isis_circuit_down()`,` circuit->u.bc.run_dr_elect[x] `will not be cleared because `circuit->u.bc.is_dr[x]` is always 0. Consequently, when switching to a P2P link, `isis_circuit_circ_type_set()` does not reset the link in a non-C_STATE_UP state, leading to subsequent accesses of `circuit->u.p2p.t_send_p2p_hello` resulting in a non-empty yet invalid address. I believe that in `isis_circuit_down()`, the LAN link should unconditionally clear `circuit->u.bc.run_dr_elect[x]`. Signed-off-by: zhou-run <zhou.run@h3c.com>

pathd: fix heap use after deletion

eb098b0

Signed-off-by: GalaxyGorilla <sascha@netdef.org>

sylane approved these changes Jun 29, 2020

View reviewed changes

GalaxyGorilla merged commit 6dee0bf into pathd-pcep-rebase Jun 29, 2020

sylane deleted the pathd-pcep-rebase-fix_mem_error branch July 22, 2020 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pathd: fix heap use after deletion #28

pathd: fix heap use after deletion #28

GalaxyGorilla commented Jun 29, 2020

GalaxyGorilla commented Jun 29, 2020

pathd: fix heap use after deletion #28

pathd: fix heap use after deletion #28

Conversation

GalaxyGorilla commented Jun 29, 2020

GalaxyGorilla commented Jun 29, 2020