Module: xenomai-3
Branch: wip/dovetail
Commit: e1b0d84ce7ab5a49bd4c5553bb9060055ebd2adb
URL:    
http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=e1b0d84ce7ab5a49bd4c5553bb9060055ebd2adb

Author: Philippe Gerum <r...@xenomai.org>
Date:   Wed Jun 29 09:54:12 2016 +0200

cobalt/thread: fix race on accessing the global thread list

nkthreadq is only used by the /proc interface handlers these days, to
get the list of active Cobalt threads.

Adding an emerging user-space thread to that list should not be done
before it is actually mated to a Cobalt shadow, otherwise the /proc
handlers might find such thread before its host_task pointer is known,
causing invalid memory accesses.

A simple way to reproduce the issue involves running a (user) thread
creation loop while continuously reading from /proc/xenomai/sched/* in
parallel.

The fix postpones the insertion of emerging threads into the global
thread queue until their TCB is fully built.

---

 kernel/cobalt/thread.c |   84 +++++++++++++++++++++++++++---------------------
 1 file changed, 47 insertions(+), 37 deletions(-)

diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index cabc6b8..435cd4b 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -88,6 +88,13 @@ static void periodic_handler(struct xntimer *timer)
        fixup_ptimer_affinity(thread);
 }
 
+static inline void enlist_new_thread(struct xnthread *thread)
+{                              /* nklock held, irqs off */
+       list_add_tail(&thread->glink, &nkthreadq);
+       cobalt_nrthreads++;
+       xnvfile_touch_tag(&nkthreadlist_tag);
+}
+
 struct kthread_arg {
        struct xnthread *thread;
        struct completion *done;
@@ -573,10 +580,10 @@ void __xnthread_discard(struct xnthread *thread)
  * Initializes a new thread. The thread is left dormant until it is
  * actually started by xnthread_start().
  *
- * @param thread The address of a thread descriptor the nucleus will
- * use to store the thread-specific data.  This descriptor must always
- * be valid while the thread is active therefore it must be allocated
- * in permanent memory. @warning Some architectures may require the
+ * @param thread The address of a thread descriptor Cobalt will use to
+ * store the thread-specific data.  This descriptor must always be
+ * valid while the thread is active therefore it must be allocated in
+ * permanent memory. @warning Some architectures may require the
  * descriptor to be properly aligned in memory; this is an additional
  * reason for descriptors not to be laid in the program stack where
  * alignement constraints might not always be satisfied.
@@ -587,14 +594,13 @@ void __xnthread_discard(struct xnthread *thread)
  *
  * - name: An ASCII string standing for the symbolic name of the
  * thread. This name is copied to a safe place into the thread
- * descriptor. This name might be used in various situations by the
- * nucleus for issuing human-readable diagnostic messages, so it is
- * usually a good idea to provide a sensible value here.  NULL is fine
- * though and means "anonymous".
+ * descriptor. This name might be used in various situations by Cobalt
+ * for issuing human-readable diagnostic messages, so it is usually a
+ * good idea to provide a sensible value here.  NULL is fine though
+ * and means "anonymous".
  *
  * - flags: A set of creation flags affecting the operation. The
- * following flags can be part of this bitmask, each of them affecting
- * the nucleus behaviour regarding the created thread:
+ * following flags can be part of this bitmask:
  *
  *   - XNSUSP creates the thread in a suspended state. In such a case,
  * the thread shall be explicitly resumed using the xnthread_resume()
@@ -606,8 +612,8 @@ void __xnthread_discard(struct xnthread *thread)
  * user-space task. Otherwise, a new kernel host task is created, then
  * paired with the new Xenomai thread.
  *
- * - XNFPU (enable FPU) tells the nucleus that the new thread may use
- * the floating-point unit. XNFPU is implicitly assumed for user-space
+ * - XNFPU (enable FPU) tells Cobalt that the new thread may use the
+ * floating-point unit. XNFPU is implicitly assumed for user-space
  * threads even if not set in @a flags.
  *
  * - affinity: The processor affinity of this thread. Passing
@@ -636,7 +642,6 @@ int xnthread_init(struct xnthread *thread,
 {
        struct xnsched *sched;
        cpumask_t affinity;
-       spl_t s;
        int ret;
 
        if (attr->flags & ~(XNFPU | XNUSER | XNSUSP))
@@ -659,12 +664,6 @@ int xnthread_init(struct xnthread *thread,
 
        trace_cobalt_thread_init(thread, attr, sched_class);
 
-       xnlock_get_irqsave(&nklock, s);
-       list_add_tail(&thread->glink, &nkthreadq);
-       cobalt_nrthreads++;
-       xnvfile_touch_tag(&nkthreadlist_tag);
-       xnlock_put_irqrestore(&nklock, s);
-
        return 0;
 }
 EXPORT_SYMBOL_GPL(xnthread_init);
@@ -684,9 +683,8 @@ EXPORT_SYMBOL_GPL(xnthread_init);
  * execution properties of the new thread. Members of this structure
  * are defined as follows:
  *
- * - mode: The initial thread mode. The following flags can
- * be part of this bitmask, each of them affecting the nucleus
- * behaviour regarding the started thread:
+ * - mode: The initial thread mode. The following flags can be part of
+ * this bitmask:
  *
  *   - XNLOCK causes the thread to lock the scheduler when it starts.
  * The target thread will have to call the xnsched_unlock()
@@ -701,7 +699,7 @@ EXPORT_SYMBOL_GPL(xnthread_init);
  * - entry: The address of the thread's body routine. In other words,
  * it is the thread entry point.
  *
- * - cookie: A user-defined opaque cookie the nucleus will pass to the
+ * - cookie: A user-defined opaque cookie Cobalt will pass to the
  * emerging thread as the sole argument of its entry point.
  *
  * @retval 0 if @a thread could be started ;
@@ -728,6 +726,14 @@ int xnthread_start(struct xnthread *thread,
        if (attr->mode & XNLOCK)
                thread->lock_count = 1;
 
+       /*
+        * A user-space thread starts immediately Cobalt-wise since we
+        * already have an underlying Linux context for it, so we can
+        * enlist it now to make it visible from the /proc interface.
+        */
+       if (xnthread_test_state(thread, XNUSER))
+               enlist_new_thread(thread);
+
        trace_cobalt_thread_start(thread);
 
        xnthread_resume(thread, XNDORMANT);
@@ -1000,20 +1006,20 @@ void xnthread_suspend(struct xnthread *thread, int mask,
         * current thread.
         *
         *  The net effect is that we are attempting to stop the
-        * shadow thread at the nucleus level, whilst this thread is
-        * actually running some code under the control of the Linux
-        * scheduler (i.e. it's relaxed).
+        * shadow thread for Cobalt, whilst this thread is actually
+        * running some code under the control of the Linux scheduler
+        * (i.e. it's relaxed).
         *
         *  To make this possible, we force the target Linux task to
         * migrate back to the Xenomai domain by sending it a
         * SIGSHADOW signal the interface libraries trap for this
         * specific internal purpose, whose handler is expected to
-        * call back the nucleus's migration service.
+        * call back Cobalt's migration service.
         *
-        * By forcing this migration, we make sure that the real-time
-        * nucleus controls, hence properly stops, the target thread
-        * according to the requested suspension condition. Otherwise,
-        * the shadow thread in secondary mode would just keep running
+        * By forcing this migration, we make sure that Cobalt
+        * controls, hence properly stops, the target thread according
+        * to the requested suspension condition. Otherwise, the
+        * shadow thread in secondary mode would just keep running
         * into the Linux domain, thus breaking the most common
         * assumptions regarding suspended threads.
         *
@@ -1878,10 +1884,10 @@ void xnthread_migrate_passive(struct xnthread *thread, 
struct xnsched *sched)
  *
  * Changes the base scheduling policy and paramaters of a thread. If
  * the thread is currently blocked, waiting in priority-pending mode
- * (XNSYNCH_PRIO) for a synchronization object to be signaled, the
- * nucleus will attempt to reorder the object's wait queue so that it
- * reflects the new sleeper's priority, unless the XNSYNCH_DREORD flag
- * has been set for the pended object.
+ * (XNSYNCH_PRIO) for a synchronization object to be signaled, Cobalt
+ * will attempt to reorder the object's wait queue so that it reflects
+ * the new sleeper's priority, unless the XNSYNCH_DREORD flag has been
+ * set for the pended object.
  *
  * @param thread The descriptor address of the affected thread. See
  * note.
@@ -2551,8 +2557,8 @@ static inline void init_kthread_info(struct xnthread 
*thread)
  * @internal
  * @brief Create a shadow thread context over a kernel task.
  *
- * This call maps a nucleus thread to the "current" Linux task running
- * in kernel space.  The priority and scheduling class of the
+ * This call maps a Cobalt core thread to the "current" Linux task
+ * running in kernel space.  The priority and scheduling class of the
  * underlying Linux task are not affected; it is assumed that the
  * caller did set them appropriately before issuing the shadow mapping
  * request.
@@ -2631,7 +2637,10 @@ int xnthread_map(struct xnthread *thread, struct 
completion *done)
        xnthread_resume(thread, XNDORMANT);
        ret = xnthread_harden();
        wakeup_parent(done);
+
        xnlock_get_irqsave(&nklock, s);
+
+       enlist_new_thread(thread);
        /*
         * Make sure xnthread_start() did not slip in from another CPU
         * while we were back from wakeup_parent().
@@ -2639,6 +2648,7 @@ int xnthread_map(struct xnthread *thread, struct 
completion *done)
        if (thread->entry == NULL)
                xnthread_suspend(thread, XNDORMANT,
                                 XN_INFINITE, XN_RELATIVE, NULL);
+
        xnlock_put_irqrestore(&nklock, s);
 
        xnthread_test_cancel();


_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
https://xenomai.org/mailman/listinfo/xenomai-git

Reply via email to