]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Lock position checking allows for safe lock unions
authorMaria Matejka <mq@ucw.cz>
Mon, 24 May 2021 11:41:23 +0000 (13:41 +0200)
committerMaria Matejka <mq@ucw.cz>
Mon, 22 Nov 2021 18:05:43 +0000 (19:05 +0100)
lib/locking.h
sysdep/unix/coroutine.c

index eb1bc8faf3571e0a02f2182aa3347abb561c139c..eef601541fd469ae9a3c209013e407f8e1281ade 100644 (file)
@@ -16,16 +16,14 @@ struct lock_order {
   struct domain_generic *the_bird;
 };
 
-#define LOCK_ORDER_DEPTH  (sizeof(struct lock_order) / sizeof(struct domain_generic *))
-
 extern _Thread_local struct lock_order locking_stack;
 extern _Thread_local struct domain_generic **last_locked;
 
 #define DOMAIN(type) struct domain__##type
 #define DEFINE_DOMAIN(type) DOMAIN(type) { struct domain_generic *type; }
 
-#define DOMAIN_NEW(type, name)  (DOMAIN(type)) { .type = domain_new(name) }
-struct domain_generic *domain_new(const char *name);
+#define DOMAIN_NEW(type, name)  (DOMAIN(type)) { .type = domain_new(name, OFFSETOF(struct lock_order, type)) }
+struct domain_generic *domain_new(const char *name, uint order);
 
 #define DOMAIN_NULL(type)   (DOMAIN(type)) {}
 
index 718475055d8c89a90f54e993d3e2034a1a88f30a..2eba142c6271cf85b0a0e72c4075a4335fe6c649 100644 (file)
  *     Locking subsystem
  */
 
+_Thread_local struct lock_order locking_stack = {};
+_Thread_local struct domain_generic **last_locked = NULL;
+
 #define ASSERT_NO_LOCK ASSERT_DIE(last_locked == NULL)
 
 struct domain_generic {
   pthread_mutex_t mutex;
+  uint order;
   struct domain_generic **prev;
   struct lock_order *locked_by;
   const char *name;
 };
 
-#define DOMAIN_INIT(_name) { .mutex = PTHREAD_MUTEX_INITIALIZER, .name = _name }
+#define DOMAIN_INIT(_name, _order) { .mutex = PTHREAD_MUTEX_INITIALIZER, .name = _name, .order = _order }
 
-static struct domain_generic the_bird_domain_gen = DOMAIN_INIT("The BIRD");
+static struct domain_generic the_bird_domain_gen = DOMAIN_INIT("The BIRD", OFFSETOF(struct lock_order, the_bird));
 
 DOMAIN(the_bird) the_bird_domain = { .the_bird = &the_bird_domain_gen };
 
 struct domain_generic *
-domain_new(const char *name)
+domain_new(const char *name, uint order)
 {
+  ASSERT_DIE(order < sizeof(struct lock_order));
   struct domain_generic *dg = xmalloc(sizeof(struct domain_generic));
-  *dg = (struct domain_generic) DOMAIN_INIT(name);
+  *dg = (struct domain_generic) DOMAIN_INIT(name, order);
   return dg;
 }
 
@@ -74,11 +79,11 @@ domain_free(struct domain_generic *dg)
   xfree(dg);
 }
 
-_Thread_local struct lock_order locking_stack = {};
-_Thread_local struct domain_generic **last_locked = NULL;
-
 void do_lock(struct domain_generic *dg, struct domain_generic **lsp)
 {
+  if ((char *) lsp - (char *) &locking_stack != dg->order)
+    bug("Trying to lock on bad position: order=%u, lsp=%p, base=%p", dg->order, lsp, &locking_stack);
+
   if (lsp <= last_locked)
     bug("Trying to lock in a bad order");
   if (*lsp)
@@ -96,6 +101,9 @@ void do_lock(struct domain_generic *dg, struct domain_generic **lsp)
 
 void do_unlock(struct domain_generic *dg, struct domain_generic **lsp)
 {
+  if ((char *) lsp - (char *) &locking_stack != dg->order)
+    bug("Trying to unlock on bad position: order=%u, lsp=%p, base=%p", dg->order, lsp, &locking_stack);
+
   if (dg->locked_by != &locking_stack)
     bug("Inconsistent domain state on unlock");
   if ((last_locked != lsp) || (*lsp != dg))