[pkgsrc/trunk]: pkgsrc/net/quagga Fix lib/thread.c implementation for list ha...

[kardel <kardel%pkgsrc.org@localhost>]

details:   https://anonhg.NetBSD.org/pkgsrc/rev/28d301cbc553
branches:  trunk
changeset: 340245:28d301cbc553
user:      kardel <kardel%pkgsrc.org@localhost>
date:      Mon Sep 30 08:57:49 2019 +0000

description:
Fix lib/thread.c implementation for list handling and thread_cancellation:

Documented upstream in: https://bugzilla.quagga.net/show_bug.cgi?id=1011

The included patches will remedy thread.c shortcomings and add proper safeguards
to detect future thread handling errors.
fixes in the patches:
 - add a name to a thread for error messages
 - add a list pointer to the thread to track list membership
 - add fast check on prev/next pointer invariants
 - add check to detect inserting of a thread already in a list
 - add check to detect deletion of a thread not in a list
 - ignore cancellation requests for the currently running thread (fixes crash)
 - fix setting of prey/next pointers on adjecant elements when a head/tail element is
   deleted

diffstat:

 net/quagga/distinfo                   |    4 +-
 net/quagga/patches/patch-lib_thread.c |  135 ++++++++++++++++++++++++++++++++++
 net/quagga/patches/patch-lib_thread.h |   23 +++++
 3 files changed, 161 insertions(+), 1 deletions(-)

diffs (182 lines):

diff -r a3dcbb5e8fb2 -r 28d301cbc553 net/quagga/distinfo
--- a/net/quagga/distinfo       Mon Sep 30 08:28:40 2019 +0000
+++ b/net/quagga/distinfo       Mon Sep 30 08:57:49 2019 +0000
@@ -1,9 +1,11 @@
-$NetBSD: distinfo,v 1.31 2018/03/01 01:09:06 gdt Exp $
+$NetBSD: distinfo,v 1.32 2019/09/30 08:57:49 kardel Exp $
 
 SHA1 (quagga-1.2.4.tar.gz) = 8e1471624f5baf54b53be6636e451824d3ef15f6
 RMD160 (quagga-1.2.4.tar.gz) = 9d4b807ce150db9d3cec1f6bf9a3f836d4973428
 SHA512 (quagga-1.2.4.tar.gz) = 3e72440bcccfd3c1a449a62b7ff8623441256399a2bee0a39fa0a19694a5a78ac909c5c2128a24735bc034ea8b0811827293b480a2584a3a4c8ae36be9cf1fcd
 Size (quagga-1.2.4.tar.gz) = 2925847 bytes
 SHA1 (patch-configure) = d3289b94f9aa871678398ccd78f655944277f03f
+SHA1 (patch-lib_thread.c) = 8e3cf21225a1ec5e94ac526b0b5a4e9e66ca4c93
+SHA1 (patch-lib_thread.h) = 1afcc4c4a30dfa2f0e182310568b5e31acec21fb
 SHA1 (patch-solaris_quagga.init.in) = 47569aaffe2713809e21ebbb76164cf50b7f983f
 SHA1 (patch-zebra_kernel__socket.c) = 82c1be406ec587d7bc4d6eec39f9daebbc34e999
diff -r a3dcbb5e8fb2 -r 28d301cbc553 net/quagga/patches/patch-lib_thread.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/net/quagga/patches/patch-lib_thread.c     Mon Sep 30 08:57:49 2019 +0000
@@ -0,0 +1,135 @@
+$NetBSD: patch-lib_thread.c,v 1.1 2019/09/30 08:57:49 kardel Exp $
+
+ - add fast check on prev/next pointer invariants
+ - add check to detect inserting of a thread already in a list
+ - add check to detect deletion of a thread not in a list
+ - ignore cancellation requests for the currently running thread (fixes crash)
+ - fix setting of prey/next pointers on adjecant elements when a head/tail element is
+   deleted
+
+--- lib/thread.c.orig  2018-02-19 21:24:55.000000000 +0000
++++ lib/thread.c
+@@ -531,6 +531,10 @@ thread_master_create ()
+       return NULL;
+     }
+ 
++  rv->ready.name = "ready";
++  rv->event.name = "event";
++  rv->unuse.name = "unuse";
++
+   rv->fd_limit = (int)limit.rlim_cur;
+   rv->read = XCALLOC (MTYPE_THREAD, sizeof (struct thread *) * rv->fd_limit);
+   if (rv->read == NULL)
+@@ -560,6 +564,11 @@ thread_master_create ()
+ static void
+ thread_list_add (struct thread_list *list, struct thread *thread)
+ {
++  if (thread->list) {
++      zlog_err("%s:%d: thread_list_add INCONSISTENCY thread %p is already linked in list %s", __FILE__, __LINE__, thread, thread->list->name);
++      assert(!thread->list);
++  }
++
+   thread->next = NULL;
+   thread->prev = list->tail;
+   if (list->tail)
+@@ -568,22 +577,57 @@ thread_list_add (struct thread_list *lis
+     list->head = thread;
+   list->tail = thread;
+   list->count++;
++  thread->list = list;
+ }
+ 
+ /* Delete a thread from the list. */
+ static struct thread *
+ thread_list_delete (struct thread_list *list, struct thread *thread)
+ {
++  if (!thread->list) {
++      zlog_err("%s:%d: thread_list_delete INCONSISTENCY thread %p is NOT linked in a list", __FILE__, __LINE__, thread);
++      assert(thread->list);
++  }
++
++  if (thread->list && thread->list != list) {
++      zlog_err("%s:%d: thread_list_delete INCONSISTENCY thread %p is linked in list %s but should be removed from list %s",
++      __FILE__, __LINE__, thread, thread->list->name, list->name);
++      assert(thread->list == list);
++  }
++
+   if (thread->next)
+     thread->next->prev = thread->prev;
+-  else
++  else {
++    if (list->tail != thread) {
++      zlog_debug("%s:%d: thread_list_delete INCONSISTENCY thread %p has no successor but list->tail points to %p in list %s",
++      __FILE__, __LINE__, thread, list->tail, list->name);
++      assert(list->tail == thread);
++    }
++
+     list->tail = thread->prev;
++    if (list->tail)
++      list->tail->next = NULL;
++  }
++
+   if (thread->prev)
+     thread->prev->next = thread->next;
+-  else
++  else {
++    if (list->head != thread) {
++      zlog_debug("%s:%d: thread_list_delete INCONSISTENCY thread %p has no predecessor but list->head points to %p in list %s",
++      __FILE__, __LINE__, thread, list->head, list->name);
++      assert(list->head == thread);
++    }
++
+     list->head = thread->next;
+-  thread->next = thread->prev = NULL;
++    if (list->head)
++      list->head->prev = NULL;
++  }
++
+   list->count--;
++
++  thread->next = thread->prev = NULL;
++  thread->list = NULL;
++
+   return thread;
+ }
+ 
+@@ -603,10 +647,12 @@ thread_add_fd (struct thread **thread_ar
+ static void
+ thread_add_unuse (struct thread *thread)
+ {
+-  thread->type = THREAD_UNUSED;
+   assert (thread->master != NULL && thread != NULL);
++  assert (thread_current != thread);
+   assert (thread->next == NULL);
+   assert (thread->prev == NULL);
++  assert (thread->list == NULL);
++  thread->type = THREAD_UNUSED;
+   thread_list_add (&thread->master->unuse, thread);
+ }
+ 
+@@ -948,7 +994,15 @@ thread_cancel (struct thread *thread)
+   struct thread_list *list = NULL;
+   struct pqueue *queue = NULL;
+   struct thread **thread_array = NULL;
+-  
++ 
++  /*
++   * we cannot cancel ourself when running.
++   * cancellation of ourself would lead to a double entry attempt
++   * into the unuse list.
++   */
++  if (thread_current == thread)
++      return;
++
+   switch (thread->type)
+     {
+     case THREAD_READ:
+@@ -1175,7 +1229,7 @@ thread_fetch (struct thread_master *m)
+       exceptfd = fd_copy_fd_set(m->exceptfd);
+       
+       /* Calculate select wait timer if nothing else to do */
+-      if (m->ready.count == 0)
++      if (thread_empty(&m->ready))
+         {
+           quagga_get_relative (NULL);
+           timer_wait = thread_timer_wait (m->timer, &timer_val);
diff -r a3dcbb5e8fb2 -r 28d301cbc553 net/quagga/patches/patch-lib_thread.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/net/quagga/patches/patch-lib_thread.h     Mon Sep 30 08:57:49 2019 +0000
@@ -0,0 +1,23 @@
+$NetBSD: patch-lib_thread.h,v 1.1 2019/09/30 08:57:49 kardel Exp $
+
+ - add a name to a thread for error messages
+ - add a list pointer to the thread to track list membership
+
+--- lib/thread.h.orig  2018-02-19 21:24:55.000000000 +0000
++++ lib/thread.h
+@@ -38,6 +38,7 @@ struct rusage_t
+ /* Linked list of thread. */
+ struct thread_list
+ {
++  const char    *name;
+   struct thread *head;
+   struct thread *tail;
+   int count;
+@@ -77,6 +78,7 @@ struct thread
+   thread_type add_type;               /* thread type */
+   struct thread *next;                /* next pointer of the thread */   
+   struct thread *prev;                /* previous pointer of the thread */
++  struct thread_list *list;   /* current list we are queued in */
+   struct thread_master *master;       /* pointer to the struct thread_master. */
+   int (*func) (struct thread *); /* event function */
+   void *arg;                  /* event argument */