Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: fix memory leak of CH hash tables
To:
tech@openbsd.org
Date:
Sat, 13 Dec 2025 13:10:11 +0100

Download raw body.

Thread
The double free fix from jsg@ made me wonder why I did not spot that one
earlier. The reason is that CH_DESTROY was not defined and nothing called
this cleanup function.

Fix memory leak of the CH tables used by the per-peer pending queues.

Define CH_DESTROY() and use it in peer_delete() via adjout_peer_free()
to cleanup the lookup tables used by the pending attribute and prefix
queues. Also rename adjout_prefix_flush_pending() to
adjout_peer_flush_pending() since that function does no longer work
with struct adjout_prefix entries.

-- 
:wq Claudio

Index: chash.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/chash.h,v
diff -u -p -r1.4 chash.h
--- chash.h	21 Nov 2025 12:19:00 -0000	1.4
+++ chash.h	13 Dec 2025 07:41:25 -0000
@@ -158,6 +158,7 @@ static const struct ch_type _name##_CH_I
 const struct ch_type *const _name##_CH_TYPE = &_name##_CH_INFO
 
 #define CH_INIT(_name, _head)		_name##_CH_INIT(_head)
+#define CH_DESTROY(_name, _head)	_name##_CH_DESTROY(_head)
 #define CH_INSERT(_name, _head, _elm, _prev)				\
 					_name##_CH_INSERT(_head, _elm, _prev)
 #define CH_REMOVE(_name, _head, _elm)	_name##_CH_REMOVE(_head, _elm)
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.331 rde.h
--- rde.h	12 Dec 2025 21:42:58 -0000	1.331
+++ rde.h	13 Dec 2025 07:49:28 -0000
@@ -748,7 +748,6 @@ void		 adjout_prefix_withdraw(struct rde
 		    struct adjout_prefix *);
 void		 adjout_prefix_destroy(struct rde_peer *,
 		    struct adjout_prefix *);
-void		 adjout_prefix_flush_pending(struct rde_peer *);
 int		 adjout_prefix_reaper(struct rde_peer *);
 void		 adjout_prefix_dump_cleanup(struct rib_context *);
 void		 adjout_prefix_dump_r(struct rib_context *);
@@ -763,6 +762,8 @@ int		 adjout_prefix_dump_subtree(struct 
 			struct adjout_prefix *, void *),
 		    void (*)(void *, uint8_t), int (*)(void *));
 void		 adjout_peer_init(struct rde_peer *);
+void		 adjout_peer_flush_pending(struct rde_peer *);
+void		 adjout_peer_free(struct rde_peer *);
 
 void		 pend_attr_done(struct pend_attr *, struct rde_peer *);
 void		 pend_eor_add(struct rde_peer *, uint8_t);
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.11 rde_adjout.c
--- rde_adjout.c	12 Dec 2025 21:42:58 -0000	1.11
+++ rde_adjout.c	13 Dec 2025 07:48:44 -0000
@@ -585,26 +585,6 @@ adjout_prefix_destroy(struct rde_peer *p
 	}
 }
 
-void
-adjout_prefix_flush_pending(struct rde_peer *peer)
-{
-	struct pend_attr *pa, *npa;
-	struct pend_prefix *pp, *npp;
-	uint8_t aid;
-
-	for (aid = AID_MIN; aid < AID_MAX; aid++) {
-		TAILQ_FOREACH_SAFE(pp, &peer->withdraws[aid], entry, npp) {
-			pend_prefix_free(pp, &peer->withdraws[aid], peer);
-		}
-		TAILQ_FOREACH_SAFE(pa, &peer->updates[aid], entry, npa) {
-			TAILQ_FOREACH_SAFE(pp, &pa->prefixes, entry, npp) {
-				pend_prefix_free(pp, &pa->prefixes, peer);
-			}
-			pend_attr_done(pa, peer);
-		}
-	}
-}
-
 int
 adjout_prefix_reaper(struct rde_peer *peer)
 {
@@ -825,4 +805,32 @@ adjout_peer_init(struct rde_peer *peer)
 		TAILQ_INIT(&peer->updates[i]);
 	for (i = 0; i < nitems(peer->withdraws); i++)
 		TAILQ_INIT(&peer->withdraws[i]);
+}
+
+void
+adjout_peer_flush_pending(struct rde_peer *peer)
+{
+	struct pend_attr *pa, *npa;
+	struct pend_prefix *pp, *npp;
+	uint8_t aid;
+
+	for (aid = AID_MIN; aid < AID_MAX; aid++) {
+		TAILQ_FOREACH_SAFE(pp, &peer->withdraws[aid], entry, npp) {
+			pend_prefix_free(pp, &peer->withdraws[aid], peer);
+		}
+		TAILQ_FOREACH_SAFE(pa, &peer->updates[aid], entry, npa) {
+			TAILQ_FOREACH_SAFE(pp, &pa->prefixes, entry, npp) {
+				pend_prefix_free(pp, &pa->prefixes, peer);
+			}
+			pend_attr_done(pa, peer);
+		}
+	}
+}
+
+void
+adjout_peer_free(struct rde_peer *peer)
+{
+	adjout_peer_flush_pending(peer);	/* not strictly needed */
+	CH_DESTROY(pend_attr_hash, &peer->pend_attrs);
+	CH_DESTROY(pend_prefix_hash, &peer->pend_prefixes);
 }
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.61 rde_peer.c
--- rde_peer.c	12 Dec 2025 21:42:58 -0000	1.61
+++ rde_peer.c	13 Dec 2025 07:50:29 -0000
@@ -427,7 +427,7 @@ peer_down(struct rde_peer *peer)
 	 * and flush all pending imsg from the SE.
 	 */
 	rib_dump_terminate(peer);
-	adjout_prefix_flush_pending(peer);
+	adjout_peer_flush_pending(peer);
 	peer_imsg_flush(peer);
 
 	/* flush Adj-RIB-In */
@@ -441,8 +441,8 @@ peer_delete(struct rde_peer *peer)
 	if (peer->state != PEER_DOWN)
 		peer_down(peer);
 
-	/* free filters */
 	filterlist_free(peer->out_rules);
+	adjout_peer_free(peer);
 
 	RB_REMOVE(peer_tree, &peertable, peer);
 	while (RB_INSERT(peer_tree, &zombietable, peer) != NULL) {
@@ -500,7 +500,7 @@ peer_stale(struct rde_peer *peer, uint8_
 	 * and flush all pending imsg from the SE.
 	 */
 	rib_dump_terminate(peer);
-	adjout_prefix_flush_pending(peer);
+	adjout_peer_flush_pending(peer);
 	peer_imsg_flush(peer);
 
 	if (flushall)