From f0507f05ce57398e135651896dace4cb68eeed54 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Mon, 27 Sep 2021 16:40:28 +0200
Subject: 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.
---
 proto/static/static.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

(limited to 'proto/static')

diff --git a/proto/static/static.c b/proto/static/static.c
index f8ac9c81..b0c9d640 100644
--- a/proto/static/static.c
+++ b/proto/static/static.c
@@ -52,11 +52,14 @@ static linpool *static_lp;
 static inline struct rte_src * static_get_source(struct static_proto *p, uint i)
 { return i ? rt_get_source(&p->p, i) : p->p.main_source; }
 
+static inline void static_free_source(struct rte_src *src, uint i)
+{ if (i) rt_unlock_source(src); }
+
 static void
 static_announce_rte(struct static_proto *p, struct static_route *r)
 {
+  struct rte_src *src;
   rta *a = allocz(RTA_MAX_SIZE);
-  struct rte_src *src = static_get_source(p, r->index);
   a->source = RTS_STATIC;
   a->scope = SCOPE_UNIVERSE;
   a->dest = r->dest;
@@ -103,6 +106,7 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
     return;
 
   /* We skip rta_lookup() here */
+  src = static_get_source(p, r->index);
   rte e0 = { .attrs = a, .src = src, .net = r->net, }, *e = &e0;
 
   /* Evaluate the filter */
@@ -110,6 +114,8 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
     f_eval_rte(r->cmds, e, static_lp);
 
   rte_update(p->p.main_channel, r->net, e, src);
+  static_free_source(src, r->index);
+
   r->state = SRS_CLEAN;
 
   if (r->cmds)
@@ -121,7 +127,9 @@ withdraw:
   if (r->state == SRS_DOWN)
     return;
 
+  src = static_get_source(p, r->index);
   rte_update(p->p.main_channel, r->net, NULL, src);
+  static_free_source(src, r->index);
   r->state = SRS_DOWN;
 }
 
@@ -287,7 +295,11 @@ static void
 static_remove_rte(struct static_proto *p, struct static_route *r)
 {
   if (r->state)
-    rte_update(p->p.main_channel, r->net, NULL, static_get_source(p, r->index));
+  {
+    struct rte_src *src = static_get_source(p, r->index);
+    rte_update(p->p.main_channel, r->net, NULL, src);
+    static_free_source(src, r->index);
+  }
 
   static_reset_rte(p, r);
 }
@@ -444,6 +456,8 @@ static_postconfig(struct proto_config *CF)
   static_index_routes(cf);
 }
 
+static struct rte_owner_class static_rte_owner_class;
+
 static struct proto *
 static_init(struct proto_config *CF)
 {
@@ -455,8 +469,7 @@ static_init(struct proto_config *CF)
 
   P->neigh_notify = static_neigh_notify;
   P->reload_routes = static_reload_routes;
-  P->rte_better = static_rte_better;
-  P->rte_mergable = static_rte_mergable;
+  P->sources.class = &static_rte_owner_class;
 
   if (cf->igp_table_ip4)
     p->igp_table_ip4 = cf->igp_table_ip4->table;
@@ -757,6 +770,11 @@ static_show(struct proto *P)
     static_show_rt(r);
 }
 
+static struct rte_owner_class static_rte_owner_class = {
+  .get_route_info =	static_get_route_info,
+  .rte_better =		static_rte_better,
+  .rte_mergable =	static_rte_mergable,
+};
 
 struct protocol proto_static = {
   .name =		"Static",
@@ -773,5 +791,4 @@ struct protocol proto_static = {
   .shutdown =		static_shutdown,
   .reconfigure =	static_reconfigure,
   .copy_config =	static_copy_config,
-  .get_route_info =	static_get_route_info,
 };
-- 
cgit v1.2.3