nhrpd: Fix nhrp_peer leak #14579
Merged
nhrpd: Fix nhrp_peer leak #14579
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
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
thereby causing a memory leak.
- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.
- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265
=================================================================
==20265==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
FRRouting#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
FRRouting#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
FRRouting#3 0x7f8026a88d3f in hash_get lib/hash.c:147
FRRouting#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
FRRouting#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
FRRouting#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
FRRouting#7 0x7f8026b62e0e in event_call lib/event.c:1969
FRRouting#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
FRRouting#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
FRRouting#10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400
=================================================================
==20400==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
FRRouting#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
FRRouting#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
FRRouting#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
FRRouting#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
FRRouting#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
FRRouting#6 0x7fb6e373ee0e in event_call lib/event.c:1969
FRRouting#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
FRRouting#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
FRRouting#9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
|
@Mergifyio backport stable/9.1 |
✅ Backports have been created
|
|
@Mergifyio backport stable/9.0 stable/8.4 stable/8.5 |
✅ Backports have been created
|
donaldsharp
added a commit
that referenced
this pull request
Jun 5, 2024
nhrpd: Fix nhrp_peer leak (backport #14579)
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.
Addressed memory leak by removing
&c->peer_notifierfrom the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion ofc->cur.peerthereby causing a memory leak.Reordered termination steps to call
vrf_terminatebeforenhrp_vc_terminate, preventing a heap-use-after-free issue whennhrp_vc_notify_delis invoked innhrp_peer_check_delete.Added an if statement to avoid passing NULL as hash to
hash_release, which leads to a SIGSEGV.The ASan leak log for reference: