Skip to content

Commit

Permalink
isisd: fix crash when deactivating ISIS adjacency on the interface.
Browse files Browse the repository at this point in the history
1. When the command "no <ip|ipv6> 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=<optimized out>) 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=<optimized out>) 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=<optimized out>) at lib/sigevent.c:262
2  <signal handler called>
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=<optimized out>) 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=<optimized out>,
   context=<optimized out>) at lib/northbound.c:1131
16 nb_callback_configuration (context=<optimized out>, 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=<optimized out>, 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=<optimized out>) 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=<optimized out>) 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=<optimized out>, envp=<optimized out>) 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>
  • Loading branch information
zhou-run authored and baozhen-H3C committed May 28, 2024
1 parent b3600d8 commit 27f3cf1
Showing 1 changed file with 0 additions and 42 deletions.
42 changes: 0 additions & 42 deletions isisd/isis_lsp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 27f3cf1

Please sign in to comment.