From 27f3cf149989193bb2f39858222ad1217fc0393c Mon Sep 17 00:00:00 2001 From: zhou-run <166502045+zhou-run@users.noreply.github.com> Date: Sat, 11 May 2024 15:30:38 +0800 Subject: [PATCH] isisd: fix crash when deactivating ISIS adjacency on the interface. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. When the command "no router isis WORD" is executed on the interface, it invokes list_delete_all_node to iterate and release the memory of all nodes in the cirtcuit->u.bc.adjdb[1] linked list. However, the nodes are not unlinked during this traversal process, leading to the call of *list->del to delete the data of the linked list nodes. 2. For ISIS, deleting the data of the linked list nodes is done by calling isis_delete_adj. Subsequently, isis_level2_adj_up will be called to iterate and query the cirtcuit->u.bc.adjdb[1] linked list. If there are many neighbors on this interface, accessing the memory of the released linked list nodes may occur. 3. Not limited to ISIS, if the linked list is not unlinked during the deletion of all nodes in process 1, *list->del should not be allowed to iterate through the list again. The backtrace is as follows: (gdb) bt at isisd/isis_csm.c:196 context=) at lib/northbound.c:1131 errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:1356 at lib/northbound.c:1473 errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:906 comment=comment@entry=0x0, transaction_id=transaction_id@entry=0x0, errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=8192) at lib/northbound.c:938 filter=FILTER_RELAXED) at lib/command.c:971 at lib/command.c:1030 vtysh=vtysh@entry=0) at lib/command.c:1198 at isisd/isis_csm.c:196 context=) at lib/northbound.c:1131 errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:1356 at lib/northbound.c:1473 errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:906 comment=comment@entry=0x0, transaction_id=transaction_id@entry=0x0, errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=8192) at lib/northbound.c:938 filter=FILTER_RELAXED) at lib/command.c:971 at lib/command.c:1030 vtysh=vtysh@entry=0) at lib/command.c:1198 0 0x00007f7d6e541fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0 1 0x00007f7d6e63188c in core_handler (signo=11, siginfo=0x7ffc0ced2630, context=) at lib/sigevent.c:262 2 3 0x00005647f5b11568 in isis_level2_adj_up (area=area@entry=0x5647f7c89830) at isisd/isis_lsp.c:423 4 0x00005647f5b14073 in isis_reset_attach_bit (adj=0x5647f7cad690) at isisd/isis_lsp.c:474 5 lsp_handle_adj_state_change (adj=0x5647f7cad690) at isisd/isis_lsp.c:2162 6 0x00005647f5b53675 in hook_call_isis_adj_state_change_hook (adj=adj@entry=0x5647f7cad690) at isisd/isis_adjacency.c:152 7 0x00005647f5b536f3 in isis_delete_adj (arg=0x5647f7cad690) at isisd/isis_adjacency.c:167 8 0x00007f7d6e5fe003 in list_delete_all_node (list=0x5647f7c88060) at lib/linklist.c:316 9 0x00007f7d6e5fe069 in list_delete (list=list@entry=0x5647f7c84708) at lib/linklist.c:326 10 0x00005647f5b0872e in isis_circuit_down (circuit=0x5647f7c84620) at isisd/isis_circuit.c:835 11 0x00005647f5b09f81 in isis_csm_state_change (event=event@entry=IF_DOWN_FROM_Z, circuit=circuit@entry=0x5647f7c84620, arg=arg@entry=0x5647f7c7f7a0) at isisd/isis_csm.c:196 12 0x00005647f5b083b0 in isis_circuit_disable (circuit=0x5647f7c84620) at isisd/isis_circuit.c:100 13 isis_circuit_del (circuit=0x5647f7c84620) at isisd/isis_circuit.c:200 14 0x00005647f5b434f5 in lib_interface_isis_destroy (args=) at isisd/isis_nb_config.c:2612 15 0x00007f7d6e61347a in nb_callback_destroy (errmsg_len=2, errmsg=0x7ffc0ced38d0 "", dnode=0x5647f7c948f0, event=NB_EV_APPLY, nb_node=, context=) at lib/northbound.c:1131 16 nb_callback_configuration (context=, event=event@entry=NB_EV_APPLY, change=change@entry=0x5647f7cb6680, errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:1356 17 0x00007f7d6e6138b7 in nb_transaction_process (errmsg_len=8192, errmsg=0x7ffc0ced38d0 "", transaction=0x5647f7c94080, event=NB_EV_APPLY) at lib/northbound.c:1473 18 nb_candidate_commit_apply (transaction=0x5647f7c94080, save_transaction=save_transaction@entry=true, transaction_id=transaction_id@entry=0x0, errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=errmsg_len@entry=8192) at lib/northbound.c:906 19 0x00007f7d6e61403d in nb_candidate_commit (context=context@entry=0x7ffc0ced38c0, candidate=, save_transaction=save_transaction@entry=true, comment=comment@entry=0x0, transaction_id=transaction_id@entry=0x0, errmsg=errmsg@entry=0x7ffc0ced38d0 "", errmsg_len=8192) at lib/northbound.c:938 20 0x00007f7d6e616ec9 in nb_cli_classic_commit (vty=0x5647f7cae160) at lib/northbound_cli.c:64 21 0x00007f7d6e6176a8 in nb_cli_apply_changes (vty=0x5647f7cae160, xpath_base_fmt=) at lib/northbound_cli.c:268 22 0x00007f7d6e5d918e in cmd_execute_command_real (vline=vline@entry=0x5647f7cae140, vty=vty@entry=0x5647f7cae160, cmd=cmd@entry=0x0, up_level=up_level@entry=0, filter=FILTER_RELAXED) at lib/command.c:971 23 0x00007f7d6e5d951d in cmd_execute_command (vline=vline@entry=0x5647f7cae140, vty=vty@entry=0x5647f7cae160, cmd=cmd@entry=0x0, vtysh=vtysh@entry=0) at lib/command.c:1030 24 0x00007f7d6e5d9770 in cmd_execute (vty=vty@entry=0x5647f7cae160, cmd=cmd@entry=0x5647f7cb48a0 "no ip router isis 10", matched=matched@entry=0x0, vtysh=vtysh@entry=0) at lib/command.c:1198 25 0x00007f7d6e6485e6 in vty_command (vty=vty@entry=0x5647f7cae160, buf=0x5647f7cb48a0 "no ip router isis 10") at lib/vty.c:483 26 0x00007f7d6e648d01 in vty_execute (vty=vty@entry=0x5647f7cae160) at lib/vty.c:1246 27 0x00007f7d6e64ba40 in vtysh_read (thread=) at lib/vty.c:2090 28 0x00007f7d6e64348d in thread_call (thread=thread@entry=0x7ffc0ced8310) at lib/thread.c:1958 29 0x00007f7d6e5fd4a8 in frr_run (master=0x5647f79a43d0) at lib/libfrr.c:1184 30 0x00005647f5b050f3 in main (argc=5, argv=, envp=) at isisd/isis_main.c:273 (gdb) f 3 423 isisd/isis_lsp.c: No such file or directory. (gdb) p node $1 = (struct listnode *) 0x110 (gdb) f 8 316 lib/linklist.c: No such file or directory. (gdb) p list->head->data $2 = (void *) 0x5647f7cabf20 (gdb) p list->head->next->data $3 = (void *) 0x5647f7c9bb60 (gdb) p list->head->next->next->data Cannot access memory at address 0x120 (gdb) p list->head->next->next $4 = (struct listnode *) 0x110 The backtrace provided above pertains to version 8.2.2, but it seems that the same issue exists in the code of the master branch as well. isis_reset_attach_bit() is useless because lsp_handle_adj_state_change() unconditionally calls lsp_regenerate_schedule. Signed-off-by: zhou-run <166502045+zhou-run@users.noreply.github.com> --- isisd/isis_lsp.c | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/isisd/isis_lsp.c b/isisd/isis_lsp.c index 9d671137e945..6cdce80c1ed8 100644 --- a/isisd/isis_lsp.c +++ b/isisd/isis_lsp.c @@ -442,47 +442,6 @@ void set_overload_on_start_timer(struct event *thread) isis_area_overload_bit_set(area, false); } -static void isis_reset_attach_bit(struct isis_adjacency *adj) -{ - struct isis_area *area = adj->circuit->area; - struct lspdb_head *head; - struct isis_lsp *lsp; - uint8_t lspid[ISIS_SYS_ID_LEN + 2]; - - /* - * If an L2 adjacency changed its state in L-1-2 area, we have to: - * - set the attached bit in L1 LSPs if it's the first L2 adjacency - * - remove the attached bit in L1 LSPs if it's the last L2 adjacency - */ - - if (area->is_type != IS_LEVEL_1_AND_2 || adj->level == ISIS_ADJ_LEVEL1) - return; - - if (!area->attached_bit_send) - return; - - head = &area->lspdb[IS_LEVEL_1 - 1]; - memset(lspid, 0, ISIS_SYS_ID_LEN + 2); - memcpy(lspid, area->isis->sysid, ISIS_SYS_ID_LEN); - - lsp = lsp_search(head, lspid); - if (!lsp) - return; - - if (adj->adj_state == ISIS_ADJ_UP - && !(lsp->hdr.lsp_bits & LSPBIT_ATT)) { - sched_debug("ISIS (%s): adj going up regenerate lsp-bits", - area->area_tag); - lsp_regenerate_schedule(area, IS_LEVEL_1, 0); - } else if (adj->adj_state == ISIS_ADJ_DOWN - && (lsp->hdr.lsp_bits & LSPBIT_ATT) - && !isis_level2_adj_up(area)) { - sched_debug("ISIS (%s): adj going down regenerate lsp-bits", - area->area_tag); - lsp_regenerate_schedule(area, IS_LEVEL_1, 0); - } -} - static uint8_t lsp_bits_generate(int level, int overload_bit, int attached_bit, struct isis_area *area) { @@ -2357,7 +2316,6 @@ static int lsp_handle_adj_state_change(struct isis_adjacency *adj) /* when an adjacency state changes determine if we need to * change attach_bits in other area's LSPs */ - isis_reset_attach_bit(adj); return 0; }