From ee72f0a0eb93038ef6dfd01fed9f32e24c5de2a1 Mon Sep 17 00:00:00 2001 From: Reuben Dowle Date: Mon, 7 Dec 2020 16:35:13 +1300 Subject: [PATCH] nhrpd: Cleanup resources when interface is deleted Currently when an interface is deleted from configuration, associated resources are not freed. This causes memory leaks and crashes. To reproduce this issue: * Connect to a DMVPN hub * Outside of frr, delete the underlying GRE interface * Use 'no interface xxx' to delete the interface containing nhrp configurations Signed-off-by: Reuben Dowle --- nhrpd/nhrp_cache.c | 42 ++++++++++++++++++++++++++++++++++++++++-- nhrpd/nhrp_interface.c | 15 +++++++++++++++ nhrpd/nhrp_nhs.c | 18 ++++++++++++++++++ nhrpd/nhrp_peer.c | 27 +++++++++++++++++++++++++++ nhrpd/nhrpd.h | 3 +++ 5 files changed, 103 insertions(+), 2 deletions(-) mode change 100644 => 100755 nhrpd/nhrp_cache.c mode change 100644 => 100755 nhrpd/nhrp_interface.c mode change 100644 => 100755 nhrpd/nhrpd.h --- a/nhrpd/nhrp_cache.c +++ b/nhrpd/nhrp_cache.c @@ -69,12 +69,13 @@ static void nhrp_cache_free(struct nhrp_ { struct nhrp_interface *nifp = c->ifp->info; - zassert(c->cur.type == NHRP_CACHE_INVALID && c->cur.peer == NULL); - zassert(c->new.type == NHRP_CACHE_INVALID && c->new.peer == NULL); + debugf(NHRP_DEBUG_COMMON, "Deleting cache entry"); nhrp_cache_counts[c->cur.type]--; notifier_call(&c->notifier_list, NOTIFY_CACHE_DELETE); zassert(!notifier_active(&c->notifier_list)); hash_release(nifp->cache_hash, c); + THREAD_OFF(c->t_timeout); + THREAD_OFF(c->t_auth); XFREE(MTYPE_NHRP_CACHE, c); } @@ -140,6 +141,41 @@ struct nhrp_cache_config *nhrp_cache_con create ? nhrp_cache_config_alloc : NULL); } +static void do_nhrp_cache_free(struct hash_bucket *hb, + void *arg __attribute__((__unused__))) +{ + struct nhrp_cache *c = hb->data; + + nhrp_cache_free(c); +} + +static void do_nhrp_cache_config_free(struct hash_bucket *hb, + void *arg __attribute__((__unused__))) +{ + struct nhrp_cache_config *cc = hb->data; + + nhrp_cache_config_free(cc); +} + +void nhrp_cache_interface_del(struct interface *ifp) +{ + struct nhrp_interface *nifp = ifp->info; + + debugf(NHRP_DEBUG_COMMON, "Cleaning up undeleted cache entries (%lu)", + nifp->cache_hash ? nifp->cache_hash->count : 0); + + if (nifp->cache_hash) { + hash_iterate(nifp->cache_hash, do_nhrp_cache_free, NULL); + hash_free(nifp->cache_hash); + } + + if (nifp->cache_config_hash) { + hash_iterate(nifp->cache_config_hash, do_nhrp_cache_config_free, + NULL); + hash_free(nifp->cache_config_hash); + } +} + struct nhrp_cache *nhrp_cache_get(struct interface *ifp, union sockunion *remote_addr, int create) { @@ -164,6 +200,7 @@ struct nhrp_cache *nhrp_cache_get(struct static int nhrp_cache_do_free(struct thread *t) { struct nhrp_cache *c = THREAD_ARG(t); + c->t_timeout = NULL; nhrp_cache_free(c); return 0; @@ -172,6 +209,7 @@ static int nhrp_cache_do_free(struct thr static int nhrp_cache_do_timeout(struct thread *t) { struct nhrp_cache *c = THREAD_ARG(t); + c->t_timeout = NULL; if (c->cur.type != NHRP_CACHE_INVALID) nhrp_cache_update_binding(c, c->cur.type, -1, NULL, 0, NULL); --- a/nhrpd/nhrp_interface.c +++ b/nhrpd/nhrp_interface.c @@ -49,6 +49,21 @@ static int nhrp_if_new_hook(struct inter static int nhrp_if_delete_hook(struct interface *ifp) { + struct nhrp_interface *nifp = ifp->info; + + debugf(NHRP_DEBUG_IF, "Deleted interface (%s)", ifp->name); + + nhrp_cache_interface_del(ifp); + nhrp_nhs_interface_del(ifp); + nhrp_peer_interface_del(ifp); + + if (nifp->ipsec_profile) + free(nifp->ipsec_profile); + if (nifp->ipsec_fallback_profile) + free(nifp->ipsec_fallback_profile); + if (nifp->source) + free(nifp->source); + XFREE(MTYPE_NHRP_IF, ifp->info); return 0; } --- a/nhrpd/nhrp_nhs.c +++ b/nhrpd/nhrp_nhs.c @@ -378,6 +378,24 @@ int nhrp_nhs_free(struct nhrp_nhs *nhs) return 0; } +void nhrp_nhs_interface_del(struct interface *ifp) +{ + struct nhrp_interface *nifp = ifp->info; + struct nhrp_nhs *nhs, *tmp; + afi_t afi; + + for (afi = 0; afi < AFI_MAX; afi++) { + debugf(NHRP_DEBUG_COMMON, "Cleaning up nhs entries (%d)", + !list_empty(&nifp->afi[afi].nhslist_head)); + + list_for_each_entry_safe(nhs, tmp, &nifp->afi[afi].nhslist_head, + nhslist_entry) + { + nhrp_nhs_free(nhs); + } + } +} + void nhrp_nhs_terminate(void) { struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT); --- a/nhrpd/nhrp_peer.c +++ b/nhrpd/nhrp_peer.c @@ -38,11 +38,17 @@ static void nhrp_packet_debug(struct zbu static void nhrp_peer_check_delete(struct nhrp_peer *p) { + char buf[2][256]; struct nhrp_interface *nifp = p->ifp->info; if (p->ref || notifier_active(&p->notifier_list)) return; + debugf(NHRP_DEBUG_COMMON, "Deleting peer ref:%d remote:%s local:%s", + p->ref, + sockunion2str(&p->vc->remote.nbma, buf[0], sizeof(buf[0])), + sockunion2str(&p->vc->local.nbma, buf[1], sizeof(buf[1]))); + THREAD_OFF(p->t_fallback); hash_release(nifp->peer_hash, p); nhrp_interface_notify_del(p->ifp, &p->ifp_notifier); @@ -185,6 +191,27 @@ static void *nhrp_peer_create(void *data return p; } +static void do_peer_hash_free(struct hash_bucket *hb, + void *arg __attribute__((__unused__))) +{ + struct nhrp_peer *p = hb->data; + nhrp_peer_check_delete(p); +} + +void nhrp_peer_interface_del(struct interface *ifp) +{ + struct nhrp_interface *nifp = ifp->info; + + debugf(NHRP_DEBUG_COMMON, "Cleaning up undeleted peer entries (%lu)", + nifp->peer_hash ? nifp->peer_hash->count : 0); + + if (nifp->peer_hash) { + hash_iterate(nifp->peer_hash, do_peer_hash_free, NULL); + assert(nifp->peer_hash->count == 0); + hash_free(nifp->peer_hash); + } +} + struct nhrp_peer *nhrp_peer_get(struct interface *ifp, const union sockunion *remote_nbma) { --- a/nhrpd/nhrpd.h +++ b/nhrpd/nhrpd.h @@ -343,6 +343,7 @@ void nhrp_nhs_foreach(struct interface * void (*cb)(struct nhrp_nhs *, struct nhrp_registration *, void *), void *ctx); +void nhrp_nhs_interface_del(struct interface *ifp); void nhrp_route_update_nhrp(const struct prefix *p, struct interface *ifp); void nhrp_route_announce(int add, enum nhrp_cache_type type, @@ -366,6 +367,7 @@ void nhrp_shortcut_foreach(afi_t afi, void nhrp_shortcut_purge(struct nhrp_shortcut *s, int force); void nhrp_shortcut_prefix_change(const struct prefix *p, int deleted); +void nhrp_cache_interface_del(struct interface *ifp); void nhrp_cache_config_free(struct nhrp_cache_config *c); struct nhrp_cache_config *nhrp_cache_config_get(struct interface *ifp, union sockunion *remote_addr, @@ -446,6 +448,7 @@ struct nhrp_reqid *nhrp_reqid_lookup(str int nhrp_packet_init(void); +void nhrp_peer_interface_del(struct interface *ifp); struct nhrp_peer *nhrp_peer_get(struct interface *ifp, const union sockunion *remote_nbma); struct nhrp_peer *nhrp_peer_ref(struct nhrp_peer *p);