Skip to content

Commit

Permalink
nhrpd: Fix nhrp_peer leak
Browse files Browse the repository at this point in the history
- 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
    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)
    #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 <[email protected]>
  • Loading branch information
Keelan10 committed Oct 12, 2023
1 parent 9bc4d9e commit d163f89
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 2 deletions.
2 changes: 2 additions & 0 deletions nhrpd/nhrp_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion nhrpd/nhrp_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion nhrpd/nhrp_peer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d163f89

Please sign in to comment.