BGP: Fix memory leak in graceful restart code

Prefix and bucket tables are initialized when entering established state
but not explicitly freed when leaving it (that is handled by protocol
restart). With graceful restart, BGP may enter and leave established
state multiple times without hard protocol restart causing memory leak.

diff --git a/lib/hash.h b/lib/hash.h
index 4239b1d8991ee495f0f92e18bef1a679f69397c4..6995bbc8f64be1ea7a2fbe4ab813a7b0416a8511 100644 (file)
--- a/lib/hash.h
+++ b/lib/hash.h
@@ -25,6 +25,12 @@
     (v).data = mb_allocz(pool, HASH_SIZE(v) * sizeof(* (v).data));     \
   })
 
+#define HASH_FREE(v)                                                   \
+  ({                                                                   \
+    mb_free((v).data);                                                 \
+    (v) = (typeof(v)){ };                                              \
+  })
+
 #define HASH_FIND(v,id,key...)                                         \
   ({                                                                   \
     u32 _h = HASH_FN(v, id, key);                                      \

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index aa2a3b4601022afb152c08e4b358aeca404b76fa..9d23374a43a948a2edfb31bf7af99016c8279ee8 100644 (file)
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -934,6 +934,15 @@ bgp_init_prefix_table(struct bgp_proto *p, u32 order)
   p->prefix_slab = sl_new(p->p.pool, sizeof(struct bgp_prefix));
 }
 
+void
+bgp_free_prefix_table(struct bgp_proto *p)
+{
+  HASH_FREE(p->prefix_hash);
+
+  rfree(p->prefix_slab);
+  p->prefix_slab = NULL;
+}
+
 static struct bgp_prefix *
 bgp_get_prefix(struct bgp_proto *p, ip_addr prefix, int pxlen, u32 path_id)
 {
@@ -1940,6 +1949,23 @@ bgp_init_bucket_table(struct bgp_proto *p)
   // fib_init(&p->prefix_fib, p->p.pool, sizeof(struct bgp_prefix), 0, bgp_init_prefix);
 }
 
+void
+bgp_free_bucket_table(struct bgp_proto *p)
+{
+  mb_free(p->bucket_hash);
+  p->bucket_hash = NULL;
+
+  struct bgp_bucket *b;
+  WALK_LIST_FIRST(b, p->bucket_queue)
+  {
+    rem_node(&b->send_node);
+    mb_free(b);
+  }
+
+  mb_free(p->withdraw_bucket);
+  p->withdraw_bucket = NULL;
+}
+
 void
 bgp_get_route_info(rte *e, byte *buf, ea_list *attrs)
 {

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 8ef4b990108b9beacddbfb0d4f21a4a43a7f7692..0f1c9446ded8c48f9365cf1b96855df896655bc9 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -416,6 +416,9 @@ bgp_conn_leave_established_state(struct bgp_proto *p)
   BGP_TRACE(D_EVENTS, "BGP session closed");
   p->conn = NULL;
 
+  bgp_free_prefix_table(p);
+  bgp_free_bucket_table(p);
+
   if (p->p.proto_state == PS_UP)
     bgp_stop(p, 0);
 }

diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index b4067f3a02aa517169fc94ff90c481645a603dc6..d028bef4427d54fb7611de4bfc31f4cc60e95fc2 100644 (file)
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -253,8 +253,10 @@ int bgp_rte_recalculate(rtable *table, net *net, rte *new, rte *old, rte *old_be
 void bgp_rt_notify(struct proto *P, rtable *tbl UNUSED, net *n, rte *new, rte *old UNUSED, ea_list *attrs);
 int bgp_import_control(struct proto *, struct rte **, struct ea_list **, struct linpool *);
 void bgp_init_bucket_table(struct bgp_proto *);
+void bgp_free_bucket_table(struct bgp_proto *p);
 void bgp_free_bucket(struct bgp_proto *p, struct bgp_bucket *buck);
 void bgp_init_prefix_table(struct bgp_proto *p, u32 order);
+void bgp_free_prefix_table(struct bgp_proto *p);
 void bgp_free_prefix(struct bgp_proto *p, struct bgp_prefix *bp);
 uint bgp_encode_attrs(struct bgp_proto *p, byte *w, ea_list *attrs, int remains);
 void bgp_get_route_info(struct rte *, byte *buf, struct ea_list *attrs);