diff options
author | Maria Matejka <mq@ucw.cz> | 2021-09-27 16:40:28 +0200 |
---|---|---|
committer | Maria Matejka <mq@ucw.cz> | 2021-11-22 19:05:44 +0100 |
commit | f0507f05ce57398e135651896dace4cb68eeed54 (patch) | |
tree | 44bfd6148689af15f4f5469b2f37bca55c3e7327 /proto | |
parent | 3b20722a1fc777c27ab2e0451d0ea3fee7fa81a2 (diff) |
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.
Diffstat (limited to 'proto')
-rw-r--r-- | proto/babel/babel.c | 16 | ||||
-rw-r--r-- | proto/bgp/attrs.c | 20 | ||||
-rw-r--r-- | proto/bgp/bgp.c | 15 | ||||
-rw-r--r-- | proto/bgp/bgp.h | 7 | ||||
-rw-r--r-- | proto/bgp/packets.c | 5 | ||||
-rw-r--r-- | proto/mrt/mrt.c | 4 | ||||
-rw-r--r-- | proto/ospf/ospf.c | 13 | ||||
-rw-r--r-- | proto/ospf/ospf.h | 2 | ||||
-rw-r--r-- | proto/pipe/pipe.c | 2 | ||||
-rw-r--r-- | proto/rip/rip.c | 25 | ||||
-rw-r--r-- | proto/static/static.c | 27 |
11 files changed, 96 insertions, 40 deletions
diff --git a/proto/babel/babel.c b/proto/babel/babel.c index 03c85b7d..40e85a16 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -2264,7 +2264,7 @@ babel_preexport(struct channel *c, struct rte *new) { struct rta *a = new->attrs; /* Reject our own unreachable routes */ - if ((a->dest == RTD_UNREACHABLE) && (new->src->proto == c->proto)) + if ((a->dest == RTD_UNREACHABLE) && (new->src->owner == &c->proto->sources)) return -1; return 0; @@ -2288,7 +2288,7 @@ babel_rt_notify(struct proto *P, struct channel *c UNUSED, const net_addr *net, uint rt_metric = ea_get_int(new->attrs->eattrs, EA_BABEL_METRIC, 0); u64 rt_router_id = 0; - if (new->src->proto == P) + if (new->src->owner == &P->sources) { rt_seqno = ea_find(new->attrs->eattrs, EA_BABEL_SEQNO)->u.data; eattr *e = ea_find(new->attrs->eattrs, EA_BABEL_ROUTER_ID); @@ -2372,6 +2372,12 @@ babel_postconfig(struct proto_config *CF) cf->ip6_channel = ip6 ?: ip6_sadr; } +static struct rte_owner_class babel_rte_owner_class = { + .get_route_info = babel_get_route_info, + .rte_better = babel_rte_better, + .rte_igp_metric = babel_rte_igp_metric, +}; + static struct proto * babel_init(struct proto_config *CF) { @@ -2385,8 +2391,8 @@ babel_init(struct proto_config *CF) P->if_notify = babel_if_notify; P->rt_notify = babel_rt_notify; P->preexport = babel_preexport; - P->rte_better = babel_rte_better; - P->rte_igp_metric = babel_rte_igp_metric; + + P->sources.class = &babel_rte_owner_class; return P; } @@ -2479,7 +2485,6 @@ babel_reconfigure(struct proto *P, struct proto_config *CF) return 1; } - struct protocol proto_babel = { .name = "Babel", .template = "babel%d", @@ -2494,6 +2499,5 @@ struct protocol proto_babel = { .start = babel_start, .shutdown = babel_shutdown, .reconfigure = babel_reconfigure, - .get_route_info = babel_get_route_info, .get_attr = babel_get_attr }; 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 diff --git a/proto/mrt/mrt.c b/proto/mrt/mrt.c index 5da3c7c6..9d78438d 100644 --- a/proto/mrt/mrt.c +++ b/proto/mrt/mrt.c @@ -472,9 +472,9 @@ mrt_rib_table_entry(struct mrt_table_dump_state *s, rte *r) #ifdef CONFIG_BGP /* Find peer index */ - if (r->src->proto->proto == &proto_bgp) + struct bgp_proto *p = bgp_rte_proto(r); + if (p) { - struct bgp_proto *p = (void *) r->src->proto; struct mrt_peer_entry *n = HASH_FIND(s->peer_hash, PEER, p->remote_id, p->remote_as, p->remote_ip); diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c index 22150590..16774df6 100644 --- a/proto/ospf/ospf.c +++ b/proto/ospf/ospf.c @@ -376,8 +376,8 @@ ospf_init(struct proto_config *CF) P->reload_routes = ospf_reload_routes; P->feed_begin = ospf_feed_begin; P->feed_end = ospf_feed_end; - P->rte_better = ospf_rte_better; - P->rte_igp_metric = ospf_rte_igp_metric; + + P->sources.class = &ospf_rte_owner_class; return P; } @@ -488,7 +488,7 @@ ospf_preexport(struct channel *c, rte *e) struct ospf_area *oa = ospf_main_area(p); /* Reject our own routes */ - if (e->src->proto == c->proto) + if (e->sender == c->in_req.hook) return -1; /* Do not export routes to stub areas */ @@ -1517,6 +1517,12 @@ ospf_sh_lsadb(struct lsadb_show_data *ld) } +struct rte_owner_class ospf_rte_owner_class = { + .get_route_info = ospf_get_route_info, + .rte_better = ospf_rte_better, + .rte_igp_metric = ospf_rte_igp_metric, +}; + struct protocol proto_ospf = { .name = "OSPF", .template = "ospf%d", @@ -1532,5 +1538,4 @@ struct protocol proto_ospf = { .reconfigure = ospf_reconfigure, .get_status = ospf_get_status, .get_attr = ospf_get_attr, - .get_route_info = ospf_get_route_info }; diff --git a/proto/ospf/ospf.h b/proto/ospf/ospf.h index 3e704ae8..a5f83e79 100644 --- a/proto/ospf/ospf.h +++ b/proto/ospf/ospf.h @@ -1007,6 +1007,8 @@ void ospf_sh_state(struct proto *P, int verbose, int reachable); void ospf_sh_lsadb(struct lsadb_show_data *ld); +extern struct rte_owner_class ospf_rte_owner_class; + /* iface.c */ void ospf_iface_chstate(struct ospf_iface *ifa, u8 state); void ospf_iface_sm(struct ospf_iface *ifa, int event); diff --git a/proto/pipe/pipe.c b/proto/pipe/pipe.c index bbcd8f0d..a30da0e2 100644 --- a/proto/pipe/pipe.c +++ b/proto/pipe/pipe.c @@ -91,7 +91,7 @@ pipe_preexport(struct channel *c, rte *e) { log_rl(&p->rl_gen, L_ERR "Route overpiped (%u hops of %u configured in %s) in table %s: %N %s/%u:%u", e->generation, max_generation, c->proto->name, - c->table->name, e->net, e->src->proto->name, e->src->private_id, e->src->global_id); + c->table->name, e->net, e->src->owner->name, e->src->private_id, e->src->global_id); return -1; } diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 0a9844f3..ee05f058 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -353,7 +353,7 @@ rip_rt_notify(struct proto *P, struct channel *ch UNUSED, const net_addr *net, s en->valid = RIP_ENTRY_VALID; en->metric = rt_metric; en->tag = rt_tag; - en->from = (new->src->proto == P) ? rt_from : NULL; + en->from = (new->src->owner == &P->sources) ? rt_from : NULL; en->iface = new->attrs->nh.iface; en->next_hop = new->attrs->nh.gw; } @@ -1082,11 +1082,20 @@ rip_reload_routes(struct channel *C) rip_kick_timer(p); } +static struct rte_owner_class rip_rte_owner_class; + +static inline struct rip_proto * +rip_rte_proto(struct rte *rte) +{ + return (rte->src->owner->class == &rip_rte_owner_class) ? + SKIP_BACK(struct rip_proto, p.sources, rte->src->owner) : NULL; +} + static int rip_rte_better(struct rte *new, struct rte *old) { ASSERT_DIE(new->src == old->src); - struct rip_proto *p = (struct rip_proto *) new->src->proto; + struct rip_proto *p = rip_rte_proto(new); u32 new_metric = ea_get_int(new->attrs->eattrs, EA_RIP_METRIC, p->infinity); u32 old_metric = ea_get_int(old->attrs->eattrs, EA_RIP_METRIC, p->infinity); @@ -1121,8 +1130,7 @@ rip_init(struct proto_config *CF) P->rt_notify = rip_rt_notify; P->neigh_notify = rip_neigh_notify; P->reload_routes = rip_reload_routes; - P->rte_better = rip_rte_better; - P->rte_igp_metric = rip_rte_igp_metric; + P->sources.class = &rip_rte_owner_class; return P; } @@ -1197,7 +1205,7 @@ rip_reconfigure(struct proto *P, struct proto_config *CF) static void rip_get_route_info(rte *rte, byte *buf) { - struct rip_proto *p = (struct rip_proto *) rte->src->proto; + struct rip_proto *p = rip_rte_proto(rte); u32 rt_metric = ea_get_int(rte->attrs->eattrs, EA_RIP_METRIC, p->infinity); u32 rt_tag = ea_get_int(rte->attrs->eattrs, EA_RIP_TAG, 0); @@ -1324,6 +1332,12 @@ rip_dump(struct proto *P) } +static struct rte_owner_class rip_rte_owner_class = { + .get_route_info = rip_get_route_info, + .rte_better = rip_rte_better, + .rte_igp_metric = rip_rte_igp_metric, +}; + struct protocol proto_rip = { .name = "RIP", .template = "rip%d", @@ -1338,6 +1352,5 @@ struct protocol proto_rip = { .start = rip_start, .shutdown = rip_shutdown, .reconfigure = rip_reconfigure, - .get_route_info = rip_get_route_info, .get_attr = rip_get_attr }; 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, }; |