commit 03b9fdf4ce26b4e39a3e755fc717fe4e5ab773dd

Author: David Smith <dsmith@redhat.com>

Date:   Thu Apr 28 10:20:47 2016 -0500

    Fix PR19954 by avoiding "suspicious RCU usage" message.

    * runtime/transport/symbols.c (_stp_module_update_self): Properly handle

      RCU locking when retrieving the 'kallsyms' member of the module

      structure.

diff --git a/runtime/transport/symbols.c b/runtime/transport/symbols.c

index cb7964f..98b0239 100644

--- a/runtime/transport/symbols.c

+++ b/runtime/transport/symbols.c

@@ -249,7 +249,12 @@ static int _stp_module_update_self (void)

 		}

 		else if (!strcmp(".symtab", attr->name)) {

 #ifdef STAPCONF_MOD_KALLSYMS

-			struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);

+			struct mod_kallsyms *kallsyms;

+

+			rcu_read_lock_sched();

+			kallsyms = rcu_dereference_sched(mod->kallsyms);

+			rcu_read_unlock_sched();

+

 			if (attr->address == (unsigned long) kallsyms->symtab)

 				_stp_module_self.sections[0].size =

 					kallsyms->num_symtab * sizeof(kallsyms->symtab[0]);

commit c8db32343c9b8012fa348cf4f8f104617960f793

Author: David Smith <dsmith@redhat.com>

Date:   Thu Apr 28 10:59:50 2016 -0500

    Improved fake utrace locking.

    * runtime/stp_utrace.c: Fixed potential locking issues by changing the

      'task_work_added' and 'report_work_added' members of 'struct

      utrace' to be atomic variables. In the process, I also renamed

      'task_work_added' to 'resume_work_added'. As atomice variables, they can

      be modified without locking the utrace struct. Also renamed the 'work'

      member of 'struct utrace' to 'resume_work' (to match up with

      'resume_work_added').

diff --git a/runtime/stp_utrace.c b/runtime/stp_utrace.c

index a8afc0d..bb2d663 100644

--- a/runtime/stp_utrace.c

+++ b/runtime/stp_utrace.c

@@ -66,17 +66,23 @@ struct utrace {

 	unsigned int vfork_stop:1; /* need utrace_stop() before vfork wait */

 	unsigned int death:1;	/* in utrace_report_death() now */

 	unsigned int reap:1;	/* release_task() has run */

-	unsigned int pending_attach:1; /* need splice_attaching() */

-	unsigned int task_work_added:1; /* called task_work_add() on 'work' */

-	unsigned int report_work_added:1; /* called task_work_add()

-					   * on 'report_work' */

+	unsigned int pending_attach:1;	/* need splice_attaching() */

+

+	/* We need the '*_work_added' variables to be atomic so they

+	 * can be modified without locking the utrace struct. This is

+	 * typically done in atomic context where we can't grab the

+	 * lock. */

+	atomic_t resume_work_added;	/* called task_work_add() on

+					 * 'resume_work' */

+	atomic_t report_work_added;	/* called task_work_add() on

+					 * 'report_work' */

 	unsigned long utrace_flags;

 	struct hlist_node hlist;       /* task_utrace_table linkage */

 	struct task_struct *task;

-	struct task_work work;

+	struct task_work resume_work;

 	struct task_work report_work;

 };

@@ -349,17 +355,20 @@ static int utrace_exit(void)

 static void

 stp_task_notify_resume(struct task_struct *target, struct utrace *utrace)

 {

-	if (! utrace->task_work_added) {

-		int rc = stp_task_work_add(target, &utrace->work);

-		if (rc == 0) {

-			utrace->task_work_added = 1;

-		}

-		/* stp_task_work_add() returns -ESRCH if the task has

-		 * already passed exit_task_work(). Just ignore this

-		 * error. */

-		else if (rc != -ESRCH) {

-			printk(KERN_ERR "%s:%d - task_work_add() returned %d\n",

-			       __FUNCTION__, __LINE__, rc);

+	if (atomic_add_unless(&utrace->resume_work_added, 1, 1)) {

+		int rc = stp_task_work_add(target, &utrace->resume_work);

+		if (rc != 0) {

+			atomic_set(&utrace->report_work_added, 0);

+

+			/* stp_task_work_add() returns -ESRCH if the

+			 * task has already passed

+			 * exit_task_work(). Just ignore this

+			 * error. */

+			if (rc != -ESRCH) {

+				printk(KERN_ERR

+				       "%s:%d - task_work_add() returned %d\n",

+				       __FUNCTION__, __LINE__, rc);

+			}

 		}

 	}

 }

@@ -394,8 +403,9 @@ static void utrace_cleanup(struct utrace *utrace)

 	    list_del(&engine->entry);

 	    kmem_cache_free(utrace_engine_cachep, engine);

 	}

+	stp_spin_unlock(&utrace->lock);

-	if (utrace->task_work_added) {

+	if (atomic_add_unless(&utrace->resume_work_added, -1, 0)) {

 #ifdef STP_TF_DEBUG

 		if (stp_task_work_cancel(utrace->task, &utrace_resume) == NULL)

 			printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",

@@ -406,9 +416,8 @@ static void utrace_cleanup(struct utrace *utrace)

 #else

 		stp_task_work_cancel(utrace->task, &utrace_resume);

 #endif

-		utrace->task_work_added = 0;

 	}

-	if (utrace->report_work_added) {

+	if (atomic_add_unless(&utrace->report_work_added, -1, 0)) {

 #ifdef STP_TF_DEBUG

 		if (stp_task_work_cancel(utrace->task, &utrace_report_work) == NULL)

 			printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",

@@ -419,9 +428,7 @@ static void utrace_cleanup(struct utrace *utrace)

 #else

 		stp_task_work_cancel(utrace->task, &utrace_report_work);

 #endif

-		utrace->report_work_added = 0;

 	}

-	stp_spin_unlock(&utrace->lock);

 	/* Free the struct utrace itself. */

 	kmem_cache_free(utrace_cachep, utrace);

@@ -522,8 +529,10 @@ static bool utrace_task_alloc(struct task_struct *task)

 	INIT_LIST_HEAD(&utrace->attaching);

 	utrace->resume = UTRACE_RESUME;

 	utrace->task = task;

-	stp_init_task_work(&utrace->work, &utrace_resume);

+	stp_init_task_work(&utrace->resume_work, &utrace_resume);

 	stp_init_task_work(&utrace->report_work, &utrace_report_work);

+	atomic_set(&utrace->resume_work_added, 0);

+	atomic_set(&utrace->report_work_added, 0);

 	stp_spin_lock(&task_utrace_lock);

 	u = __task_utrace_struct(task);

@@ -558,8 +567,8 @@ static void utrace_free(struct utrace *utrace)

 	stp_spin_unlock(&task_utrace_lock);

 	/* Free the utrace struct. */

-	stp_spin_lock(&utrace->lock);

 #ifdef STP_TF_DEBUG

+	stp_spin_lock(&utrace->lock);

 	if (unlikely(utrace->reporting)

 	    || unlikely(!list_empty(&utrace->attached))

 	    || unlikely(!list_empty(&utrace->attaching)))

@@ -567,27 +576,31 @@ static void utrace_free(struct utrace *utrace)

 		       __FUNCTION__, __LINE__, utrace->reporting,

 		       list_empty(&utrace->attached),

 		       list_empty(&utrace->attaching));

+	stp_spin_unlock(&utrace->lock);

 #endif

-	if (utrace->task_work_added) {

-		if (stp_task_work_cancel(utrace->task, &utrace_resume) == NULL)

-			printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",

+	if (atomic_add_unless(&utrace->resume_work_added, -1, 0)) {

+		if ((stp_task_work_cancel(utrace->task, &utrace_resume)

+		     == NULL)

+		    && (utrace->task->flags & ~PF_EXITING)

+		    && (utrace->task->exit_state == 0))

+			printk(KERN_ERR "%s:%d * task_work_cancel() failed? task %p, %d, %s, 0x%lx 0x%x\n",

 			       __FUNCTION__, __LINE__, utrace->task,

 			       utrace->task->tgid,

 			       (utrace->task->comm ? utrace->task->comm

-				: "UNKNOWN"));

-		utrace->task_work_added = 0;

-	}

-	if (utrace->report_work_added) {

-		if (stp_task_work_cancel(utrace->task, &utrace_report_work) == NULL)

-			printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",

+				: "UNKNOWN"), utrace->task->state, utrace->task->exit_state);

+	}

+	if (atomic_add_unless(&utrace->report_work_added, -1, 0)) {

+		if ((stp_task_work_cancel(utrace->task, &utrace_report_work)

+		     == NULL)

+		    && (utrace->task->flags & ~PF_EXITING)

+		    && (utrace->task->exit_state == 0))

+			printk(KERN_ERR "%s:%d ** task_work_cancel() failed? task %p, %d, %s, 0x%lx, 0x%x\n",

 			       __FUNCTION__, __LINE__, utrace->task,

 			       utrace->task->tgid,

 			       (utrace->task->comm ? utrace->task->comm

-				: "UNKNOWN"));

-		utrace->report_work_added = 0;

+				: "UNKNOWN"), utrace->task->state, utrace->task->exit_state);

 	}

-	stp_spin_unlock(&utrace->lock);

 	kmem_cache_free(utrace_cachep, utrace);

 }

@@ -2257,7 +2270,7 @@ static void utrace_report_death(void *cb_data __attribute__ ((unused)),

 	 * of detach bookkeeping.

 	 */

 	if (in_atomic() || irqs_disabled()) {

-		if (! utrace->report_work_added) {

+		if (atomic_add_unless(&utrace->report_work_added, 1, 1)) {

 			int rc;

 #ifdef STP_TF_DEBUG

 			printk(KERN_ERR "%s:%d - adding task_work\n",

@@ -2265,17 +2278,17 @@ static void utrace_report_death(void *cb_data __attribute__ ((unused)),

 #endif

 			rc = stp_task_work_add(task,

 					       &utrace->report_work);

-			if (rc == 0) {

-				utrace->report_work_added = 1;

-			}

-			/* stp_task_work_add() returns -ESRCH if the

-			 * task has already passed

-			 * exit_task_work(). Just ignore this

-			 * error. */

-			else if (rc != -ESRCH) {

-				printk(KERN_ERR

-				       "%s:%d - task_work_add() returned %d\n",

-				       __FUNCTION__, __LINE__, rc);

+			if (rc != 0) {

+				atomic_set(&utrace->report_work_added, 0);

+				/* stp_task_work_add() returns -ESRCH

+				 * if the task has already passed

+				 * exit_task_work(). Just ignore this

+				 * error. */

+				if (rc != -ESRCH) {

+					printk(KERN_ERR

+					       "%s:%d - task_work_add() returned %d\n",

+					       __FUNCTION__, __LINE__, rc);

+				}

 			}

 		}

 	}

@@ -2337,13 +2350,13 @@ static void utrace_resume(struct task_work *work)

 	 * instantaneous (where 'task_utrace_struct()' has to do a

 	 * hash lookup).

 	 */

-	struct utrace *utrace = container_of(work, struct utrace, work);

+	struct utrace *utrace = container_of(work, struct utrace, resume_work);

 	struct task_struct *task = current;

 	INIT_REPORT(report);

 	struct utrace_engine *engine;

 	might_sleep();

-	utrace->task_work_added = 0;

+	atomic_set(&utrace->resume_work_added, 0);

 	/* Make sure the task isn't exiting. */

 	if (task->flags & PF_EXITING) {

@@ -2436,8 +2449,8 @@ static void utrace_report_work(struct task_work *work)

 	       __FUNCTION__, __LINE__, in_atomic(), irqs_disabled());

 #endif

 	might_sleep();

-	utrace->report_work_added = 0;

+	atomic_set(&utrace->report_work_added, 0);

 	stp_spin_lock(&utrace->lock);

 	BUG_ON(utrace->death);

 	utrace->death = 1;

commit 0859b50e3ac4782e53c4c10b82c6e24174378bd4

Author: Mateusz Guzik <mguzik@redhat.com>

Date:   Mon May 2 12:28:55 2016 -0500

    Plug preempt leak in _stp_runtime_entryfn_put/get_context.

    If _stp_runtime_entryfn_get_context returns a context, preemption

    counter is always incremented. On the other hand

    _stp_runtime_entryfn_put_context only decrements the counter if the

    passed context matches the one currently set on the cpu.

    The context can be set to NULL by _stp_runtime_contexts_free, making the

    comparison false and in effect leading to a leak, e.g.:

    timer: _stp_ctl_work_callback+0x0/0x1e0[stap_af8544c7eb51251ef8c

     377abff659b05_25070] preempt leak: 00000101 -> 00000102

diff --git a/runtime/linux/runtime_context.h b/runtime/linux/runtime_context.h

index c9ffe18..9d325da 100644

--- a/runtime/linux/runtime_context.h

+++ b/runtime/linux/runtime_context.h

@@ -80,11 +80,12 @@ static struct context * _stp_runtime_entryfn_get_context(void)

 static inline void _stp_runtime_entryfn_put_context(struct context *c)

 {

-	if (c && c == _stp_runtime_get_context()) {

-		atomic_dec(&c->busy);

+	if (c) {

+		if (c == _stp_runtime_get_context())

+			atomic_dec(&c->busy);

+		/* else, warn about bad state? */

 		preempt_enable_no_resched();

 	}

-	/* else, warn about bad state? */

 	return;

 }

commit 0beafcec9a2971d466419d430d13fdb2c4f50d94

Author: David Smith <dsmith@redhat.com>

Date:   Tue May 3 13:23:58 2016 -0500

    Fix PR20040 by keeping the task_exe_file function from sleeping.

    * tapset/linux/task.stp: No longer call get_task_mm()/mmput(), so that the

      function won't sleep and cause kernel bugs.

diff --git a/tapset/linux/task.stp b/tapset/linux/task.stp

index 9b204f5..774cf58 100644

--- a/tapset/linux/task.stp

+++ b/tapset/linux/task.stp

@@ -795,13 +795,14 @@ function task_exe_file:long(task:long)

 		STAP_ERROR ("invalid task struct pointer");

 	}

+	// We'd like to call get_task_mm()/mmput() here, but they can

+	// sleep. So, let's hope incrementing the task's usage (by

+	// calling get_task_struct) is enough to keep the mm around.

 	get_task_struct(task);

-	mm = get_task_mm(task);

-	put_task_struct(task);

-	if (mm) {

+	mm = task->mm;

+	if (mm)

 		exe_file = stap_find_exe_file(mm);

-		mmput(mm);

-	}

+	put_task_struct(task);

 	if (exe_file) {

 		STAP_RETURN((unsigned long)exe_file);