Commit f0507f05 authored by Maria Matejka's avatar Maria Matejka
Browse files

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.
parent 3b20722a
Loading
Loading
Loading
Loading
+3 −2
Original line number Diff line number Diff line
@@ -526,7 +526,7 @@
      case SA_FROM:	RESULT(sa.f_type, ip, rta->from); break;
      case SA_GW:	RESULT(sa.f_type, ip, rta->nh.gw); break;
      case SA_NET:	RESULT(sa.f_type, net, fs->rte->net); break;
      case SA_PROTO:	RESULT(sa.f_type, s, fs->rte->src->proto->name); break;
      case SA_PROTO:	RESULT(sa.f_type, s, fs->rte->src->owner->name); break;
      case SA_SOURCE:	RESULT(sa.f_type, i, rta->source); break;
      case SA_SCOPE:	RESULT(sa.f_type, i, rta->scope); break;
      case SA_DEST:	RESULT(sa.f_type, i, rta->dest); break;
@@ -562,7 +562,8 @@
	{
	  ip_addr ip = v1.val.ip;
	  struct iface *ifa = ipa_is_link_local(ip) ? rta->nh.iface : NULL;
	  neighbor *n = neigh_find(fs->rte->src->proto, ip, ifa, 0);
	  /* XXX this code supposes that every owner is a protocol XXX */
	  neighbor *n = neigh_find(SKIP_BACK(struct proto, sources, fs->rte->src->owner), ip, ifa, 0);
	  if (!n || (n->scope == SCOPE_HOST))
	    runtime( "Invalid gw address" );

+1 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ struct lock_order {
  struct domain_generic *the_bird;
  struct domain_generic *proto;
  struct domain_generic *rtable;
  struct domain_generic *attrs;
  struct domain_generic *cork;
  struct domain_generic *event;
};
+9 −3
Original line number Diff line number Diff line
@@ -2329,10 +2329,17 @@ channel_reset_limit(struct channel *c, struct limit *l, int dir)
  c->limit_active &= ~(1 << dir);
}

static struct rte_owner_class default_rte_owner_class;

static inline void
proto_do_start(struct proto *p)
{
  p->active = 1;

  rt_init_sources(&p->sources, p->name, proto_event_list(p));
  if (!p->sources.class)
    p->sources.class = &default_rte_owner_class;

  if (!p->cf->late_if_feed)
    if_feed_baby(p);
}
@@ -2341,10 +2348,8 @@ static void
proto_do_up(struct proto *p)
{
  if (!p->main_source)
  {
    p->main_source = rt_get_source(p, 0);
    rt_lock_source(p->main_source);
  }
    // Locked automaticaly

  proto_start_channels(p);

@@ -2371,6 +2376,7 @@ proto_do_stop(struct proto *p)
  }

  proto_stop_channels(p);
  rt_destroy_sources(&p->sources, p->event);

  p->do_stop = 1;
  proto_send_event(p);
+2 −2
Original line number Diff line number Diff line
@@ -78,7 +78,6 @@ struct protocol {
  int (*start)(struct proto *);			/* Start the instance */
  int (*shutdown)(struct proto *);		/* Stop the instance */
  void (*get_status)(struct proto *, byte *buf); /* Get instance status (for `show protocols' command) */
  void (*get_route_info)(struct rte *, byte *buf); /* Get route information (for `show route' command) */
  int (*get_attr)(const struct eattr *, byte *buf, int buflen);	/* ASCIIfy dynamic attribute (returns GA_*) */
  void (*show_proto_info)(struct proto *);	/* Show protocol info (for `show protocols all' command) */
  void (*copy_config)(struct proto_config *, struct proto_config *);	/* Copy config from given protocol instance */
@@ -146,6 +145,7 @@ struct proto {
  list channels;			/* List of channels to rtables (struct channel) */
  struct channel *main_channel;		/* Primary channel */
  struct rte_src *main_source;		/* Primary route source */
  struct rte_owner sources;		/* Route source owner structure */
  struct iface *vrf;			/* Related VRF instance, NULL if global */

  const char *name;				/* Name of this instance (== cf->name) */
@@ -360,7 +360,7 @@ void proto_notify_state(struct proto *p, unsigned state);
 */

static inline int proto_is_inactive(struct proto *p)
{ return (p->active_channels == 0) && (p->active_coroutines == 0); }
{ return (p->active_channels == 0) && (p->active_coroutines == 0) && (p->sources.uc == 0); }


/*
+55 −7
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@
#include "lib/bitmap.h"
#include "lib/resource.h"
#include "lib/net.h"
#include "lib/hash.h"
#include "lib/event.h"

#include <stdatomic.h>

@@ -579,10 +581,10 @@ struct nexthop {

struct rte_src {
  struct rte_src *next;			/* Hash chain */
  struct proto *proto;			/* Protocol the source is based on */
  struct rte_owner *owner;		/* Route source owner */
  u32 private_id;			/* Private ID, assigned by the protocol */
  u32 global_id;			/* Globally unique ID of the source */
  unsigned uc;				/* Use count */
  _Atomic u64 uc;			/* Use count */
};


@@ -720,11 +722,57 @@ typedef struct ea_list {
#define EALF_BISECT 2			/* Use interval bisection for searching */
#define EALF_CACHED 4			/* Attributes belonging to cached rta */

struct rte_src *rt_find_source(struct proto *p, u32 id);
struct rte_src *rt_get_source(struct proto *p, u32 id);
static inline void rt_lock_source(struct rte_src *src) { src->uc++; }
static inline void rt_unlock_source(struct rte_src *src) { src->uc--; }
void rt_prune_sources(void);
struct rte_owner_class {
  void (*get_route_info)(struct rte *, byte *buf); /* Get route information (for `show route' command) */
  int (*rte_better)(struct rte *, struct rte *);
  int (*rte_mergable)(struct rte *, struct rte *);
  u32 (*rte_igp_metric)(struct rte *);
};

struct rte_owner {
  struct rte_owner_class *class;
  int (*rte_recalculate)(struct rtable *, struct network *, struct rte *, struct rte *, struct rte *);
  HASH(struct rte_src) hash;
  const char *name;
  u32 hash_key;
  u32 uc;
  event_list *list;
  event *prune;
  event *stop;
};

DEFINE_DOMAIN(attrs);
extern DOMAIN(attrs) attrs_domain;

#define RTA_LOCK	LOCK_DOMAIN(attrs, attrs_domain)
#define RTA_UNLOCK	UNLOCK_DOMAIN(attrs, attrs_domain)

#define RTE_SRC_PU_SHIFT      44
#define RTE_SRC_IN_PROGRESS   (1ULL << RTE_SRC_PU_SHIFT)

struct rte_src *rt_get_source_o(struct rte_owner *o, u32 id);
#define rt_get_source(p, id)  rt_get_source_o(&(p)->sources, (id))
static inline void rt_lock_source(struct rte_src *src)
{
  u64 uc = atomic_fetch_add_explicit(&src->uc, 1, memory_order_acq_rel);
  ASSERT_DIE(uc > 0);
}

static inline void rt_unlock_source(struct rte_src *src)
{
  u64 uc = atomic_fetch_add_explicit(&src->uc, RTE_SRC_IN_PROGRESS, memory_order_acq_rel);
  u64 pending = uc >> RTE_SRC_PU_SHIFT;
  uc &= RTE_SRC_IN_PROGRESS - 1;

  ASSERT_DIE(uc > pending);
  if (uc == pending + 1)
    ev_send(src->owner->list, src->owner->prune);

  atomic_fetch_sub_explicit(&src->uc, RTE_SRC_IN_PROGRESS + 1, memory_order_acq_rel);
}

void rt_init_sources(struct rte_owner *, const char *name, event_list *list);
void rt_destroy_sources(struct rte_owner *, event *);

struct ea_walk_state {
  ea_list *eattrs;			/* Ccurrent ea_list, initially set by caller */
Loading