diff options
| author | Keelan10 <keelan.cannoo@icloud.com> | 2023-10-12 18:04:52 +0400 |
|---|---|---|
| committer | Mergify <37929162+mergify[bot]@users.noreply.github.com> | 2024-06-05 13:57:22 +0000 |
| commit | f1c19c9c0f49fbb53ff9c08e24a0e283b812a107 (patch) | |
| tree | c9414a38e3fc766aab2b16af7fc44ac4f0b81fe6 | |
| parent | 1df49faf538643deae764da81265b912a870767c (diff) | |
nhrpd: Fix nhrp_peer leak
- 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)
#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
#3 0x7f8026a88d3f in hash_get lib/hash.c:147
#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
#7 0x7f8026b62e0e in event_call lib/event.c:1969
#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
#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)
#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
#6 0x7fb6e373ee0e in event_call lib/event.c:1969
#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
#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>
(cherry picked from commit d163f89e14ea265cd3bbd310276a8ed5ac1fb2ae)
| -rw-r--r-- | nhrpd/nhrp_cache.c | 2 | ||||
| -rw-r--r-- | nhrpd/nhrp_main.c | 2 | ||||
| -rw-r--r-- | nhrpd/nhrp_peer.c | 3 |
3 files changed, 5 insertions, 2 deletions
diff --git a/nhrpd/nhrp_cache.c b/nhrpd/nhrp_cache.c index 1a11e0d98b..e0b8f7bf88 100644 --- a/nhrpd/nhrp_cache.c +++ b/nhrpd/nhrp_cache.c @@ -70,6 +70,8 @@ static void nhrp_cache_free(struct nhrp_cache *c) notifier_call(&c->notifier_list, NOTIFY_CACHE_DELETE); assert(!notifier_active(&c->notifier_list)); hash_release(nifp->cache_hash, c); + if (c->cur.peer) + nhrp_peer_notify_del(c->cur.peer, &c->peer_notifier); nhrp_peer_unref(c->cur.peer); nhrp_peer_unref(c->new.peer); EVENT_OFF(c->t_timeout); diff --git a/nhrpd/nhrp_main.c b/nhrpd/nhrp_main.c index 593498ca13..e401f21ed4 100644 --- a/nhrpd/nhrp_main.c +++ b/nhrpd/nhrp_main.c @@ -88,8 +88,8 @@ static void nhrp_request_stop(void) nhrp_zebra_terminate(); vici_terminate(); evmgr_terminate(); - nhrp_vc_terminate(); vrf_terminate(); + nhrp_vc_terminate(); debugf(NHRP_DEBUG_COMMON, "Done."); frr_fini(); diff --git a/nhrpd/nhrp_peer.c b/nhrpd/nhrp_peer.c index ffb6cf7506..ef92d81436 100644 --- a/nhrpd/nhrp_peer.c +++ b/nhrpd/nhrp_peer.c @@ -45,7 +45,8 @@ static void nhrp_peer_check_delete(struct nhrp_peer *p) EVENT_OFF(p->t_fallback); EVENT_OFF(p->t_timer); - hash_release(nifp->peer_hash, p); + if (nifp->peer_hash) + hash_release(nifp->peer_hash, p); nhrp_interface_notify_del(p->ifp, &p->ifp_notifier); nhrp_vc_notify_del(p->vc, &p->vc_notifier); XFREE(MTYPE_NHRP_PEER, p); |
