-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
isisd: fix crash when obtaining the next hop to calculate LFA on LAN links #16303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…links When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in area->adjacency_list may be inconsistent with the neighbors in lsp->tlvs->oldstyle_reach/extended_reach. For example, the area->adjacency_list may lack certain neighbors even though they are present in the LSP. When computing SPF, the call to isis_spf_build_adj_list() generates the spftree->sadj_list, which reflects the real neighbors in the area->adjacency_list. However, in the case of LAN links, spftree->sadj_list may include additional pseudo neighbors. The pre-loading of tents through the call to isis_spf_preload_tent involves two steps: 1. isis_spf_process_lsp() is called to generate real neighbor vertices based on the root LSP and pseudo LSP. 2. isis_spf_add_local() is called to add corresponding next hops to the vertex->Adj_N list for the real neighbor vertices. In the case of LAN links, the absence of corresponding real neighbors in the spftree->sadj_list prevents the execution of the second step. Consequently, the vertex->Adj_N list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access when isis_lfa_compute() is called to calculate LFA. As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue. The backtrace is as follows: (gdb) bt #0 0x00007fd065277fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x00007fd065398972 in core_handler (signo=11, siginfo=0x7ffc5c0636b0, context=0x7ffc5c063580) at ../lib/sigevent.c:261 #2 <signal handler called> #3 0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134 #4 0x00005564d82f8d78 in isis_spf_run_lfa (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_lfa.c:2344 #5 0x00005564d8315964 in isis_run_spf_with_protection (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_spf.c:1827 #6 0x00005564d8315c15 in isis_run_spf_cb (thread=0x7ffc5c064590) at ../isisd/isis_spf.c:1889 #7 0x00007fd0653b1f04 in thread_call (thread=0x7ffc5c064590) at ../lib/thread.c:1990 #8 0x00007fd06534a97b in frr_run (master=0x5564d88103c0) at ../lib/libfrr.c:1198 #9 0x00005564d82e7d5d in main (argc=5, argv=0x7ffc5c0647b8, envp=0x7ffc5c0647e8) at ../isisd/isis_main.c:273 (gdb) f 3 #3 0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134 2134 ../isisd/isis_lfa.c: No such file or directory. (gdb) p vadj_primary $1 = (struct isis_vertex_adj *) 0x0 (gdb) p vertex->Adj_N->head $2 = (struct listnode *) 0x0 (gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->data $8 = (struct isis_vertex *) 0x5564d8b5b240 (gdb) p $8->type $9 = VTYPE_NONPSEUDO_TE_IS (gdb) p $8->N.id $10 = "\000\000\000\000\000\002" (gdb) p $8->Adj_N->count $11 = 0 (gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->next->data $12 = (struct isis_vertex *) 0x5564d8b73dd0 (gdb) p $12->type $13 = VTYPE_NONPSEUDO_TE_IS (gdb) p $12->N.id $14 = "\000\000\000\000\000\003" (gdb) p $12->Adj_N->count $15 = 0 (gdb) p area->adjacency_list->count $16 = 0 The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well. The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes isis_vertex_adj_del() to delete the next hop of a vertex. Upon reviewing all the instances where the vertex->Adj_N list is used, I found that only isis_lfa_compute() lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, the vertex->parents list for IP vertices is guaranteed not to be empty. Test scenario: Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue. Signed-off-by: zhou-run <[email protected]>
@Mergifyio backport dev/10.1 stable/10.0 stable/9.1 stable/9.0 |
🟠 Pending
|
The topotest for OSPF often fails. Should we consider extending the convergence time for testing? @ton31337 |
riw777
approved these changes
Jul 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
Jafaral
added a commit
that referenced
this pull request
Jul 2, 2024
isisd: fix crash when obtaining the next hop to calculate LFA on LAN links (backport #16303)
donaldsharp
added a commit
that referenced
this pull request
Jul 3, 2024
isisd: fix crash when obtaining the next hop to calculate LFA on LAN links (backport #16303)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in
area->adjacency_list
may be inconsistent with the neighbors inlsp->tlvs->oldstyle_reach/extended_reach
. For example, thearea->adjacency_list
may lack certain neighbors even though they are present in the LSP. When computing SPF, the call toisis_spf_build_adj_list()
generates thespftree->sadj_list
, which reflects the real neighbors in thearea->adjacency_list
. However, in the case of LAN links,spftree->sadj_list
may include additional pseudo neighbors. The pre-loading of tents through the call toisis_spf_preload_tent()
involves two steps:isis_spf_process_lsp()
is called to generate real neighbor vertices based on the root LSP and pseudo LSP.isis_spf_add_local()
is called to add corresponding next hops to thevertex->Adj_N
list for the real neighbor vertices.In the case of LAN links, the absence of corresponding real neighbors in the
spftree->sadj_list
prevents the execution of the second step. Consequently, thevertex->Adj_N
list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access whenisis_lfa_compute()
is called to calculate LFA.As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue.
The backtrace is as follows:
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes
isis_vertex_adj_del()
to delete the next hop of a vertex. Upon reviewing all the instances where thevertex->Adj_N
list is used, I found that onlyisis_lfa_compute()
lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, thevertex->parents
list for IP vertices is guaranteed not to be empty.Test scenario:
Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue.
Signed-off-by: zhou-run [email protected]