Resource pools are now bound with domains.

Memory allocation is a fragile part of BIRD and we need checking that
everybody is using the resource pools in an appropriate way. To assure
this, all the resource pools are associated with locking domains and
every resource manipulation is thoroughly checked whether the
appropriate locking domain is locked.

With transitive resource manipulation like resource dumping or mass free
operations, domains are locked and unlocked on the go, thus we require
pool domains to have higher order than their parent to allow for this
transitive operations.

Adding pool locking revealed some cases of insecure memory manipulation
and this commit fixes that as well.

5 files changed, 124 insertions, 54 deletions

diff --git a/lib/hash_test.c b/lib/hash_test.c
index ecfcdd66..30213320 100644
--- a/lib/hash_test.c
+++ b/lib/hash_test.c
@@ -61,7 +61,7 @@ dump_nodes(void)
 static void
 init_hash_(uint order)
 {
-  my_pool = rp_new(&root_pool, "Test pool");
+  my_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Test pool");
 
   HASH_INIT(hash, my_pool, order);
 
diff --git a/lib/io-loop.h b/lib/io-loop.h
index b0d3d6cc..80cd2ea2 100644
--- a/lib/io-loop.h
+++ b/lib/io-loop.h
@@ -31,6 +31,7 @@ event_list *birdloop_event_list(struct birdloop *loop);
 
 /* Get birdloop's time heap */
 struct timeloop *birdloop_time_loop(struct birdloop *loop);
+#define birdloop_domain(l)  (birdloop_time_loop((l))->domain)
 
 /* Get birdloop's pool */
 pool *birdloop_pool(struct birdloop *loop);
diff --git a/lib/locking.h b/lib/locking.h
index 6bed14bf..751eecbd 100644
--- a/lib/locking.h
+++ b/lib/locking.h
@@ -13,8 +13,8 @@ struct domain_generic;
 
 /* Here define the global lock order; first to last. */
 struct lock_order {
-  struct domain_generic *meta;
   struct domain_generic *the_bird;
+  struct domain_generic *meta;
   struct domain_generic *control;
   struct domain_generic *proto;
   struct domain_generic *service;
diff --git a/lib/resource.c b/lib/resource.c
index b1b89bdf..2071a411 100644
--- a/lib/resource.c
+++ b/lib/resource.c
@@ -46,6 +46,16 @@ static struct resclass pool_class = {
 
 pool root_pool;
 
+static void
+rp_init(pool *z, struct domain_generic *dom, const char *name)
+{
+  ASSERT_DIE(DG_IS_LOCKED(dom));
+
+  z->name = name;
+  z->domain = dom;
+  z->inside = (TLIST_LIST(resource)) {};
+}
+
 /**
  * rp_new - create a resource pool
  * @p: parent pool
@@ -55,77 +65,106 @@ pool root_pool;
  * parent pool.
  */
 pool *
-rp_new(pool *p, const char *name)
+rp_new(pool *p, struct domain_generic *dom, const char *name)
 {
   pool *z = ralloc(p, &pool_class);
-  z->name = name;
-  init_list(&z->inside);
+
+  if (dg_order(p->domain) > dg_order(dom))
+    bug("Requested reverse order pool creation: %s (%d) can't be a parent of %s (%d)",
+	domain_name(p->domain), dg_order(p->domain),
+	domain_name(dom), dg_order(dom));
+
+  if ((dg_order(p->domain) == dg_order(dom)) && (p->domain != dom))
+    bug("Requested incomparable order pool creation: %s (%d) can't be a parent of %s (%d)",
+	domain_name(p->domain), dg_order(p->domain),
+	domain_name(dom), dg_order(dom));
+
+  rp_init(z, dom, name);
   return z;
 }
 
 pool *
-rp_vnewf(pool *p, const char *fmt, va_list args)
+rp_vnewf(pool *p, struct domain_generic *dom, const char *fmt, va_list args)
 {
-  pool *z = rp_new(p, NULL);
+  pool *z = rp_new(p, dom, NULL);
   z->name = mb_vsprintf(p, fmt, args);
   return z;
 }
 
 pool *
-rp_newf(pool *p, const char *fmt, ...)
+rp_newf(pool *p, struct domain_generic *dom, const char *fmt, ...)
 {
   va_list args;
   va_start(args, fmt);
-  pool *z = rp_vnewf(p, fmt, args);
+  pool *z = rp_vnewf(p, dom, fmt, args);
   va_end(args);
 
   return z;
 }
 
+#define POOL_LOCK				\
+  struct domain_generic *dom = p->domain;	\
+  int locking = !DG_IS_LOCKED(dom);		\
+  if (locking)					\
+    DG_LOCK(dom);				\
+
+#define POOL_UNLOCK if (locking) DG_UNLOCK(dom);\
+
+void rp_free(pool *p)
+{
+  ASSERT_DIE(DG_IS_LOCKED(p->domain));
+  rfree(p);
+}
 
 static void
 pool_free(resource *P)
 {
   pool *p = (pool *) P;
-  resource *r, *rr;
 
-  r = HEAD(p->inside);
-  while (rr = (resource *) r->n.next)
+  POOL_LOCK;
+  WALK_TLIST_DELSAFE(resource, r, &p->inside)
     {
       r->class->free(r);
       xfree(r);
-      r = rr;
     }
+  POOL_UNLOCK;
 }
 
+
 static void
 pool_dump(resource *P, unsigned indent)
 {
   pool *p = (pool *) P;
-  resource *r;
+
+  POOL_LOCK;
 
   debug("%s\n", p->name);
-  WALK_LIST(r, p->inside)
+  WALK_TLIST_DELSAFE(resource, r, &p->inside)
     rdump(r, indent + 3);
+
+  POOL_UNLOCK;
 }
 
 static struct resmem
 pool_memsize(resource *P)
 {
   pool *p = (pool *) P;
-  resource *r;
   struct resmem sum = {
     .effective = 0,
     .overhead = sizeof(pool) + ALLOC_OVERHEAD,
   };
 
-  WALK_LIST(r, p->inside)
+  POOL_LOCK;
+
+  WALK_TLIST(resource, r, &p->inside)
   {
     struct resmem add = rmemsize(r);
     sum.effective += add.effective;
     sum.overhead += add.overhead;
   }
 
+  POOL_UNLOCK;
+
   return sum;
 }
 
@@ -133,14 +172,25 @@ static resource *
 pool_lookup(resource *P, unsigned long a)
 {
   pool *p = (pool *) P;
-  resource *r, *q;
+  resource *q = NULL;
+
+  POOL_LOCK;
 
-  WALK_LIST(r, p->inside)
+  WALK_TLIST(resource, r, &p->inside)
     if (r->class->lookup && (q = r->class->lookup(r, a)))
-      return q;
-  return NULL;
+      break;
+
+  POOL_UNLOCK;
+  return q;
+}
+
+static pool *
+resource_parent(resource *r)
+{
+  return SKIP_BACK(pool, inside, resource_enlisted(r));
 }
 
+
 /**
  * rmove - move a resource
  * @res: resource
@@ -152,13 +202,13 @@ pool_lookup(resource *P, unsigned long a)
 void rmove(void *res, pool *p)
 {
   resource *r = res;
+  pool *orig = resource_parent(r);
 
-  if (r)
-    {
-      if (r->n.next)
-        rem_node(&r->n);
-      add_tail(&p->inside, &r->n);
-    }
+  ASSERT_DIE(DG_IS_LOCKED(orig->domain));
+  ASSERT_DIE(DG_IS_LOCKED(p->domain));
+
+  resource_rem_node(&orig->inside, r);
+  resource_add_tail(&p->inside, r);
 }
 
 /**
@@ -179,8 +229,10 @@ rfree(void *res)
   if (!r)
     return;
 
-  if (r->n.next)
-    rem_node(&r->n);
+  pool *orig = resource_parent(r);
+  ASSERT_DIE(DG_IS_LOCKED(orig->domain));
+  resource_rem_node(&orig->inside, r);
+
   r->class->free(r);
   r->class = NULL;
   xfree(r);
@@ -239,12 +291,14 @@ rmemsize(void *res)
 void *
 ralloc(pool *p, struct resclass *c)
 {
+  ASSERT_DIE(DG_IS_LOCKED(p->domain));
+
   resource *r = xmalloc(c->size);
   bzero(r, c->size);
 
   r->class = c;
-  if (p)
-    add_tail(&p->inside, &r->n);
+  resource_add_tail(&p->inside, r);
+
   return r;
 }
 
@@ -284,28 +338,29 @@ resource_init(void)
   rcu_init();
   resource_sys_init();
 
-  root_pool.r.class = &pool_class;
-  root_pool.name = "Root";
-  init_list(&root_pool.inside);
-  tmp_init(&root_pool);
+  rp_init(&root_pool, the_bird_domain.the_bird, "Root");
+  tmp_init(&root_pool, the_bird_domain.the_bird);
 }
 
 _Thread_local struct tmp_resources tmp_res;
 
 void
-tmp_init(pool *p)
+tmp_init(pool *p, struct domain_generic *dom)
 {
   tmp_res.lp = lp_new_default(p);
   tmp_res.parent = p;
-  tmp_res.pool = rp_new(p, "TMP");
+  tmp_res.pool = rp_new(p, dom, "TMP");
+  tmp_res.domain = dom;
 }
 
 void
 tmp_flush(void)
 {
+  ASSERT_DIE(DG_IS_LOCKED(tmp_res.domain));
+
   lp_flush(tmp_linpool);
   rp_free(tmp_res.pool);
-  tmp_res.pool = rp_new(tmp_res.parent, "TMP");
+  tmp_res.pool = rp_new(tmp_res.parent, tmp_res.domain, "TMP");
 }
 
 
@@ -385,11 +440,13 @@ static struct resclass mb_class = {
 void *
 mb_alloc(pool *p, unsigned size)
 {
+  ASSERT_DIE(DG_IS_LOCKED(p->domain));
+
   struct mblock *b = xmalloc(sizeof(struct mblock) + size);
 
   b->r.class = &mb_class;
-  b->r.n = (node) {};
-  add_tail(&p->inside, &b->r.n);
+  b->r.n = (struct resource_node) {};
+  resource_add_tail(&p->inside, &b->r);
   b->size = size;
   return b->data;
 }
@@ -434,10 +491,14 @@ void *
 mb_realloc(void *m, unsigned size)
 {
   struct mblock *b = SKIP_BACK(struct mblock, data, m);
+  struct pool *p = resource_parent(&b->r);
+
+  ASSERT_DIE(DG_IS_LOCKED(p->domain));
 
   b = xrealloc(b, sizeof(struct mblock) + size);
-  update_node(&b->r.n);
   b->size = size;
+
+  resource_update_node(&p->inside, &b->r);
   return b->data;
 }
 
diff --git a/lib/resource.h b/lib/resource.h
index 2adb9de0..810334c1 100644
--- a/lib/resource.h
+++ b/lib/resource.h
@@ -10,7 +10,8 @@
 #ifndef _BIRD_RESOURCE_H_
 #define _BIRD_RESOURCE_H_
 
-#include "lib/lists.h"
+#include "lib/locking.h"
+#include "lib/tlists.h"
 
 #include <stdarg.h>
 
@@ -21,11 +22,20 @@ struct resmem {
 
 /* Resource */
 
+#define TLIST_PREFIX resource
+#define TLIST_TYPE struct resource
+#define TLIST_ITEM n
+#define TLIST_WANT_WALK
+#define TLIST_WANT_ADD_TAIL
+#define TLIST_WANT_UPDATE_NODE
+
 typedef struct resource {
-  node n;				/* Inside resource pool */
-  struct resclass *class;		/* Resource class */
+  TLIST_DEFAULT_NODE;			/* Inside resource pool */
+  const struct resclass *class;		/* Resource class */
 } resource;
 
+#include "lib/tlists.h"
+
 /* Resource class */
 
 struct resclass {
@@ -44,14 +54,13 @@ struct resclass {
 
 typedef struct pool {
   resource r;
-  list inside;
+  TLIST_LIST(resource) inside;
+  struct domain_generic *domain;
   const char *name;
 } pool;
 
 
 void resource_init(void);
-pool *rp_new(pool *, const char *);	/* Create new pool */
-pool *rp_newf(pool *, const char *, ...);	/* Create a new pool with a formatted string as its name */
 void rfree(void *);			/* Free single resource */
 void rdump(void *, unsigned indent);	/* Dump to debug output */
 struct resmem rmemsize(void *res);		/* Return size of memory used by the resource */
@@ -60,12 +69,10 @@ void rmove(void *, pool *);		/* Move to a different pool */
 
 void *ralloc(pool *, struct resclass *);
 
-pool *rp_new(pool *, const char *);		/* Create a new pool */
-pool *rp_newf(pool *, const char *, ...);	/* Create a new pool with a formatted string as its name */
-pool *rp_vnewf(pool *, const char *, va_list);	/* Create a new pool with a formatted string as its name */
-void rp_init(pool *, const char *);		/* Init a new pool */
-void rp_initf(pool *, const char *, ...);	/* Init a new pool with a formatted string as its name */
-static inline void rp_free(pool *p) { rfree(&p->r); }	/* Free the whole pool */
+pool *rp_new(pool *, struct domain_generic *, const char *);		/* Create a new pool */
+pool *rp_newf(pool *, struct domain_generic *, const char *, ...);	/* Create a new pool with a formatted string as its name */
+pool *rp_vnewf(pool *, struct domain_generic *, const char *, va_list);	/* Create a new pool with a formatted string as its name */
+void rp_free(pool *p);							/* Free the whole pool */
 
 extern pool root_pool;
 
@@ -97,6 +104,7 @@ void lp_restore(linpool *m, lp_state *p);	/* Restore state */
 struct tmp_resources {
   pool *pool, *parent;
   linpool *lp;
+  struct domain_generic *domain;
 };
 
 extern _Thread_local struct tmp_resources tmp_res;
@@ -106,7 +114,7 @@ extern _Thread_local struct tmp_resources tmp_res;
 #define tmp_allocu(sz)	lp_allocu(tmp_linpool, sz)
 #define tmp_allocz(sz)	lp_allocz(tmp_linpool, sz)
 
-void tmp_init(pool *p);
+void tmp_init(pool *p, struct domain_generic *dg);
 void tmp_flush(void);