Route sources have an explicit owner

This commit prevents use-after-free of routes belonging to protocols
which have been already destroyed, delaying also all the protocols'
shutdown until all of their routes have been finally propagated through
all the pipes down to the appropriate exports.

The use-after-free was somehow hypothetic yet theoretically possible in
rare conditions, when one BGP protocol authors a lot of routes and the
user deletes that protocol by reconfiguring in the same time as next hop
update is requested, causing rte_better() to be called on a
not-yet-pruned network prefix while the owner protocol has been already
freed.

In parallel execution environments, this would happen an inter-thread
use-after-free, causing possible heisenbugs or other nasty problems.

4 files changed, 31 insertions, 16 deletions

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 892b26e3..9b9013f9 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1670,9 +1670,8 @@ bgp_free_prefix(struct bgp_channel *c, struct bgp_prefix *px)
 int
 bgp_preexport(struct channel *c, rte *e)
 {
-  struct proto *SRC = e->src->proto;
   struct bgp_proto *p = (struct bgp_proto *) (c->proto);
-  struct bgp_proto *src = (SRC->proto == &proto_bgp) ? (struct bgp_proto *) SRC : NULL;
+  struct bgp_proto *src = bgp_rte_proto(e);
 
   /* Reject our routes */
   if (src == p)
@@ -1725,8 +1724,7 @@ bgp_preexport(struct channel *c, rte *e)
 static ea_list *
 bgp_update_attrs(struct bgp_proto *p, struct bgp_channel *c, rte *e, ea_list *attrs0, struct linpool *pool)
 {
-  struct proto *SRC = e->src->proto;
-  struct bgp_proto *src = (SRC->proto == &proto_bgp) ? (void *) SRC : NULL;
+  struct bgp_proto *src = bgp_rte_proto(e);
   struct bgp_export_state s = { .proto = p, .channel = c, .pool = pool, .src = src, .route = e, .mpls = c->desc->mpls };
   ea_list *attrs = attrs0;
   eattr *a;
@@ -1880,7 +1878,7 @@ bgp_get_neighbor(rte *r)
     return as;
 
   /* If AS_PATH is not defined, we treat rte as locally originated */
-  struct bgp_proto *p = (void *) r->src->proto;
+  struct bgp_proto *p = bgp_rte_proto(r);
   return p->cf->confederation ?: p->local_as;
 }
 
@@ -1910,8 +1908,8 @@ rte_stale(rte *r)
 int
 bgp_rte_better(rte *new, rte *old)
 {
-  struct bgp_proto *new_bgp = (struct bgp_proto *) new->src->proto;
-  struct bgp_proto *old_bgp = (struct bgp_proto *) old->src->proto;
+  struct bgp_proto *new_bgp = bgp_rte_proto(new);
+  struct bgp_proto *old_bgp = bgp_rte_proto(old);
   eattr *x, *y;
   u32 n, o;
 
@@ -2055,8 +2053,8 @@ bgp_rte_better(rte *new, rte *old)
 int
 bgp_rte_mergable(rte *pri, rte *sec)
 {
-  struct bgp_proto *pri_bgp = (struct bgp_proto *) pri->src->proto;
-  struct bgp_proto *sec_bgp = (struct bgp_proto *) sec->src->proto;
+  struct bgp_proto *pri_bgp = bgp_rte_proto(pri);
+  struct bgp_proto *sec_bgp = bgp_rte_proto(sec);
   eattr *x, *y;
   u32 p, s;
 
@@ -2137,8 +2135,8 @@ same_group(rte *r, u32 lpref, u32 lasn)
 static inline int
 use_deterministic_med(struct rte_storage *r)
 {
-  struct proto *P = r->rte.src->proto;
-  return (P->proto == &proto_bgp) && ((struct bgp_proto *) P)->cf->deterministic_med;
+  struct bgp_proto *p = bgp_rte_proto(&r->rte);
+  return p && p->cf->deterministic_med;
 }
 
 int
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 5f2e7dfd..dc845550 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -1726,6 +1726,13 @@ done:
   return p->p.proto_state;
 }
 
+struct rte_owner_class bgp_rte_owner_class = {
+  .get_route_info = 	bgp_get_route_info,
+  .rte_better =		bgp_rte_better,
+  .rte_mergable =	bgp_rte_mergable,
+  .rte_igp_metric =	bgp_rte_igp_metric,
+};
+
 static struct proto *
 bgp_init(struct proto_config *CF)
 {
@@ -1739,10 +1746,9 @@ bgp_init(struct proto_config *CF)
   P->reload_routes = bgp_reload_routes;
   P->feed_begin = bgp_feed_begin;
   P->feed_end = bgp_feed_end;
-  P->rte_better = bgp_rte_better;
-  P->rte_mergable = bgp_rte_mergable;
-  P->rte_recalculate = cf->deterministic_med ? bgp_rte_recalculate : NULL;
-  P->rte_igp_metric = bgp_rte_igp_metric;
+
+  P->sources.class = &bgp_rte_owner_class;
+  P->sources.rte_recalculate = cf->deterministic_med ? bgp_rte_recalculate : NULL;
 
   p->cf = cf;
   p->is_internal = (cf->local_as == cf->remote_as);
@@ -2605,6 +2611,5 @@ struct protocol proto_bgp = {
   .copy_config = 	bgp_copy_config,
   .get_status = 	bgp_get_status,
   .get_attr = 		bgp_get_attr,
-  .get_route_info = 	bgp_get_route_info,
   .show_proto_info = 	bgp_show_proto_info
 };
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 342dc023..7cb4df1f 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -523,6 +523,7 @@ rta_resolvable(rta *a)
   return a->dest == RTD_UNICAST;
 }
 
+extern struct rte_owner_class bgp_rte_owner_class;
 
 #ifdef LOCAL_DEBUG
 #define BGP_FORCE_DEBUG 1
@@ -594,6 +595,12 @@ int bgp_get_attr(const struct eattr *e, byte *buf, int buflen);
 void bgp_get_route_info(struct rte *, byte *);
 int bgp_total_aigp_metric_(rta *a, u64 *metric, const struct adata **ad);
 
+static inline struct bgp_proto *bgp_rte_proto(struct rte *rte)
+{
+  return (rte->src->owner->class == &bgp_rte_owner_class) ?
+    SKIP_BACK(struct bgp_proto, p.sources, rte->src->owner) : NULL;
+}
+
 #define BGP_AIGP_METRIC		1
 #define BGP_AIGP_MAX		U64(0xffffffffffffffff)
 
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index 647551e5..d2d5b174 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -1339,6 +1339,8 @@ bgp_rte_update(struct bgp_parse_state *s, net_addr *n, u32 path_id, rta *a0)
 {
   if (path_id != s->last_id)
   {
+    rt_unlock_source(s->last_src);
+
     s->last_src = rt_get_source(&s->proto->p, path_id);
     s->last_id = path_id;
 
@@ -2421,6 +2423,7 @@ bgp_decode_nlri(struct bgp_parse_state *s, u32 afi, byte *nlri, uint len, ea_lis
 
   s->last_id = 0;
   s->last_src = s->proto->p.main_source;
+  rt_lock_source(s->last_src);
 
   /*
    * IPv4 BGP and MP-BGP may be used together in one update, therefore we do not
@@ -2451,6 +2454,8 @@ bgp_decode_nlri(struct bgp_parse_state *s, u32 afi, byte *nlri, uint len, ea_lis
 
   rta_free(s->cached_rta);
   s->cached_rta = NULL;
+
+  rt_unlock_source(s->last_src);
 }
 
 static void