DragonFlyBSD Kernel Audit
DF-2870 / fix.diff
← back to finding ↓ download raw
--- a/sys/sys/taskqueue.h
+++ b/sys/sys/taskqueue.h
@@ -55,11 +55,14 @@
  */
 typedef void (*taskqueue_enqueue_fn)(void *context);
 
+#define	TASKQ_RUNNING	0x0001		/* ta_func currently executing */
+
 struct task {
 	STAILQ_ENTRY(task) ta_link;	/* link for queue */
 	struct taskqueue *ta_queue;	/* taskqueue enqueued on */
 	int	ta_pending;		/* count times queued */
 	int	ta_priority;		/* priority of task in queue */
+	int	ta_flags;		/* TASK_* flags, see above */
 	task_fn_t *ta_func;		/* task handler */
 	void	*ta_context;		/* argument for handler */
 };
@@ -99,6 +102,7 @@
 	{ .ta_queue = NULL,				\
 	  .ta_pending = 0,				\
 	  .ta_priority = (priority),			\
+	  .ta_flags = 0,					\
 	  .ta_func = (func),				\
 	  .ta_context = (context) }
 
@@ -115,6 +119,7 @@
 	(task)->ta_queue = NULL;			\
 	(task)->ta_pending = 0;				\
 	(task)->ta_priority = (priority);		\
+	(task)->ta_flags = 0;				\
 	(task)->ta_func = (func);			\
 	(task)->ta_context = (context);			\
 } while (0)
--- a/sys/kern/subr_taskqueue.c
+++ b/sys/kern/subr_taskqueue.c
@@ -54,7 +54,6 @@
 	taskqueue_enqueue_fn	tq_enqueue;
 	void			*tq_context;
 
-	struct task		*tq_running;
 	struct spinlock		tq_lock;
 	struct thread		**tq_threads;
 	int			tq_tcount;
@@ -152,6 +151,14 @@
 {
 	TQ_LOCK(queue);
 	queue->tq_flags &= ~TQ_FLAGS_ACTIVE;
+	/*
+	 * Wait for any armed timeout-task callouts to complete so they
+	 * cannot fire into the queue after it is freed.  Completing
+	 * callouts try to enqueue their task, which is rejected with
+	 * EPIPE because TQ_FLAGS_ACTIVE is already clear.
+	 */
+	while (queue->tq_callouts != 0)
+		TQ_SLEEP(queue, &queue->tq_callouts, "tqfree");
 	taskqueue_run(queue, 1);
 	taskqueue_terminate(queue->tq_threads, queue);
 	TQ_UNLOCK(queue);
@@ -325,6 +332,7 @@
 {
 	struct taskqueue *queue;
 	struct timeout_task *timeout_task;
+	int wake_freer;
 
 	timeout_task = arg;
 	queue = timeout_task->t.ta_queue;
@@ -333,8 +341,11 @@
 	KASSERT((timeout_task->f & DT_CALLOUT_ARMED) != 0, ("Stray timeout"));
 	timeout_task->f &= ~DT_CALLOUT_ARMED;
 	queue->tq_callouts--;
+	wake_freer = (queue->tq_callouts == 0);
 	taskqueue_enqueue_locked(queue, &timeout_task->t);
 	TQ_UNLOCK(queue);
+	if (wake_freer)
+		wakeup(&queue->tq_callouts);
 }
 
 int
@@ -398,17 +409,28 @@
 	while (STAILQ_FIRST(&queue->tq_queue)) {
 		/*
 		 * Carefully remove the first task from the queue and
-		 * zero its pending count.
+		 * zero its pending count.  Mark it TASKQ_RUNNING under the
+		 * queue spinlock so that concurrent runners (multi-thread
+		 * queues, taskqueue_free() racing the service thread) each
+		 * track their own task.
 		 */
 		task = STAILQ_FIRST(&queue->tq_queue);
 		STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link);
 		pending = task->ta_pending;
 		task->ta_pending = 0;
-		queue->tq_running = task;
+		atomic_set_int(&task->ta_flags, TASKQ_RUNNING);
 
 		TQ_UNLOCK(queue);
 		task->ta_func(task->ta_context, pending);
-		queue->tq_running = NULL;
+		/*
+		 * Clear TASKQ_RUNNING *before* wakeup() and while holding
+		 * the queue spinlock: sleepers check the flag under the
+		 * same spinlock and interlock their sleep with it
+		 * (ssleep), so the wakeup cannot be lost.
+		 */
+		TQ_LOCK(queue);
+		atomic_clear_int(&task->ta_flags, TASKQ_RUNNING);
+		TQ_UNLOCK(queue);
 		wakeup(task);
 		TQ_LOCK(queue);
 	}
@@ -426,7 +448,7 @@
 	if (pendp != NULL)
 		*pendp = task->ta_pending;
 	task->ta_pending = 0;
-	return (task == queue->tq_running ? EBUSY : 0);
+	return ((task->ta_flags & TASKQ_RUNNING) ? EBUSY : 0);
 }
 
 int
@@ -490,7 +512,8 @@
 taskqueue_drain(struct taskqueue *queue, struct task *task)
 {
 	TQ_LOCK(queue);
-	while (task->ta_pending != 0 || task == queue->tq_running)
+	while (task->ta_pending != 0 ||
+	       (task->ta_flags & TASKQ_RUNNING) != 0)
 		TQ_SLEEP(queue, task, "-");
 	TQ_UNLOCK(queue);
 }
@@ -509,7 +532,8 @@
 		if (queue == NULL)
 			return;
 		TQ_LOCK(queue);
-		if (task->ta_pending == 0 && task != queue->tq_running) {
+		if (task->ta_pending == 0 &&
+		    (task->ta_flags & TASKQ_RUNNING) == 0) {
 			TQ_UNLOCK(queue);
 			return;
 		}
@@ -616,6 +640,15 @@
 	TQ_LOCK(tq);
 	while ((tq->tq_flags & TQ_FLAGS_ACTIVE) != 0) {
 		taskqueue_run(tq, 1);
+		/*
+		 * Re-check TQ_FLAGS_ACTIVE (under the spinlock we still
+		 * hold from taskqueue_run) before parking.  Otherwise a
+		 * wakeup() delivered by taskqueue_terminate() while we
+		 * were inside ta_func is lost and both threads sleep
+		 * forever.
+		 */
+		if ((tq->tq_flags & TQ_FLAGS_ACTIVE) == 0)
+			break;
 		TQ_SLEEP(tq, tq, "tqthr");
 	}