0001-task_finder2-fix-task-worker-race-on-module-unload.patch

From 6cb54128e005d1220a7b064ee42b9f72561c28e7 Mon Sep 17 00:00:00 2001
From: Sultan Alsawaf <sultan@openresty.com>
Date: Wed, 30 Dec 2020 15:47:58 -0800
Subject: [PATCH] task_finder2: fix task worker race on module unload

Unfortunately, __stp_tf_cancel_all_task_work() does not guarantee that
all of the task finder's task workers will be finished executing when it
returns. In this case, we rely on the stp_task_work API to prevent the
module from being unloaded while there are task workers in-flight, which
works, but the stp_task_work API is notified of a task worker finishing
before it actually finishes. Inside __stp_tf_task_worker_fn(), the
call to the task worker's function (tf_work->func) is where the final
refcount in the stp_task_work API could be put, but there will still be
instructions left in the task worker that will be executing for a short
time after that. In that short time, there can be a race where the
module is unloaded before the task worker finishes executing all of its
instructions, especially if the task worker gets preempted during this
time on a PREEMPT kernel.

To remedy this, we must ensure that the last instruction in
__stp_tf_task_worker_fn() is where the stp_task_work API is notified of
a task worker finishing.
---
 runtime/linux/task_finder2.c | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/runtime/linux/task_finder2.c b/runtime/linux/task_finder2.c
index 83fc17b5e..2bab19295 100644
--- a/runtime/linux/task_finder2.c
+++ b/runtime/linux/task_finder2.c
@@ -150,6 +150,7 @@ __stp_tf_task_worker_fn(struct task_work *work)
 	 * workers for this task.
 	 */
 	__stp_tf_task_work_free(work);
+	stp_task_work_func_done();
 }
 
 static void
@@ -1066,11 +1067,8 @@ __stp_tf_clone_worker(struct task_work *work)
 
 	might_sleep();
 	if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING
-	    || current->flags & PF_EXITING) {
-		/* Remember that this task_work_func is finished. */
-		stp_task_work_func_done();
+	    || current->flags & PF_EXITING)
 		return;
-	}
 
 	__stp_tf_handler_start();
 
@@ -1085,10 +1083,6 @@ __stp_tf_clone_worker(struct task_work *work)
 	}
 
 	__stp_tf_handler_end();
-
-	/* Remember that this task_work_func is finished. */
-	stp_task_work_func_done();
-	return;
 }
 
 
@@ -1392,11 +1386,8 @@ __stp_tf_quiesce_worker(struct task_work *work)
 
 	might_sleep();
 	if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING
-	    || current->flags & PF_EXITING) {
-		/* Remember that this task_work_func is finished. */
-		stp_task_work_func_done();
+	    || current->flags & PF_EXITING)
 		return;
-	}
 
         /* If we had a build-id based executable probe (so we have a
          * tgt->build_id) set, we could not check it back in
@@ -1420,8 +1411,6 @@ __stp_tf_quiesce_worker(struct task_work *work)
                           (long) current->tgid, ok);
                 if (!ok) {
                         // stap_utrace_detach (current, & tgt->ops);
-                        /* Remember that this task_work_func is finished. */
-                        stp_task_work_func_done();
                         return;
                 }
         } 
@@ -1444,10 +1433,6 @@ __stp_tf_quiesce_worker(struct task_work *work)
 	__stp_call_callbacks(tgt, current, 1, (current->pid == current->tgid));
 
 	__stp_tf_handler_end();
-
-	/* Remember that this task_work_func is finished. */
-	stp_task_work_func_done();
-	return;
 }
 
 static u32
@@ -1614,18 +1599,12 @@ __stp_tf_mmap_worker(struct task_work *work)
 
 	// See if we can find saved syscall info.
 	entry = __stp_tf_get_map_entry(current);
-	if (entry == NULL) {
-		/* Remember that this task_work_func is finished. */
-		stp_task_work_func_done();
+	if (entry == NULL)
 		return;
-	}
 
 	if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING
 	    || current->flags & PF_EXITING) {
 		__stp_tf_remove_map_entry(entry);
-
-		/* Remember that this task_work_func is finished. */
-		stp_task_work_func_done();
 		return;
 	}
 
@@ -1650,10 +1629,6 @@ __stp_tf_mmap_worker(struct task_work *work)
 	__stp_tf_remove_map_entry(entry);
 
 	__stp_tf_handler_end();
-
-	/* Remember that this task_work_func is finished. */
-	stp_task_work_func_done();
-	return;
 }
 
 static u32
-- 
2.29.2