Fix use-after-free in taskq_seq_show_impl

[mirror_spl.git] / module / spl / spl-taskq.c

diff --git a/module/spl/spl-taskq.c b/module/spl/spl-taskq.c
index b362bef548ef434c978e8a7260004499c16a33b9..4298b3c86e3ed6a918928a41d0cd7eae48e471eb 100644 (file)
--- a/module/spl/spl-taskq.c
+++ b/module/spl/spl-taskq.c
@@ -50,6 +50,9 @@ MODULE_PARM_DESC(spl_taskq_thread_sequential,
 /* Global system-wide dynamic task queue available for all consumers */
 taskq_t *system_taskq;
 EXPORT_SYMBOL(system_taskq);
+/* Global dynamic task queue for long delay */
+taskq_t *system_delay_taskq;
+EXPORT_SYMBOL(system_delay_taskq);
 
 /* Private dedicated taskq for creating new taskq threads on demand. */
 static taskq_t *dynamic_taskq;
@@ -334,19 +337,18 @@ taskq_find_list(taskq_t *tq, struct list_head *lh, taskqid_t id)
 
 /*
  * Find an already dispatched task given the task id regardless of what
- * state it is in.  If a task is still pending or executing it will be
- * returned and 'active' set appropriately.  If the task has already
- * been run then NULL is returned.
+ * state it is in.  If a task is still pending it will be returned.
+ * If a task is executing, then -EBUSY will be returned instead.
+ * If the task has already been run then NULL is returned.
  */
 static taskq_ent_t *
-taskq_find(taskq_t *tq, taskqid_t id, int *active)
+taskq_find(taskq_t *tq, taskqid_t id)
 {
        taskq_thread_t *tqt;
        struct list_head *l;
        taskq_ent_t *t;
 
        ASSERT(spin_is_locked(&tq->tq_lock));
-       *active = 0;
 
        t = taskq_find_list(tq, &tq->tq_delay_list, id);
        if (t)
@@ -363,9 +365,12 @@ taskq_find(taskq_t *tq, taskqid_t id, int *active)
        list_for_each(l, &tq->tq_active_list) {
                tqt = list_entry(l, taskq_thread_t, tqt_active_list);
                if (tqt->tqt_id == id) {
-                       t = tqt->tqt_task;
-                       *active = 1;
-                       return (t);
+                       /*
+                        * Instead of returning tqt_task, we just return a non
+                        * NULL value to prevent misuse, since tqt_task only
+                        * has two valid fields.
+                        */
+                       return (ERR_PTR(-EBUSY));
                }
        }
 
@@ -402,12 +407,11 @@ taskq_find(taskq_t *tq, taskqid_t id, int *active)
 static int
 taskq_wait_id_check(taskq_t *tq, taskqid_t id)
 {
-       int active = 0;
        int rc;
        unsigned long flags;
 
        spin_lock_irqsave_nested(&tq->tq_lock, flags, tq->tq_lock_class);
-       rc = (taskq_find(tq, id, &active) == NULL);
+       rc = (taskq_find(tq, id) == NULL);
        spin_unlock_irqrestore(&tq->tq_lock, flags);
 
        return (rc);
@@ -494,15 +498,14 @@ int
 taskq_cancel_id(taskq_t *tq, taskqid_t id)
 {
        taskq_ent_t *t;
-       int active = 0;
        int rc = ENOENT;
        unsigned long flags;
 
        ASSERT(tq);
 
        spin_lock_irqsave_nested(&tq->tq_lock, flags, tq->tq_lock_class);
-       t = taskq_find(tq, id, &active);
-       if (t && !active) {
+       t = taskq_find(tq, id);
+       if (t && t != ERR_PTR(-EBUSY)) {
                list_del_init(&t->tqent_list);
                t->tqent_flags |= TQENT_FLAG_CANCEL;
 
@@ -533,7 +536,7 @@ taskq_cancel_id(taskq_t *tq, taskqid_t id)
        }
        spin_unlock_irqrestore(&tq->tq_lock, flags);
 
-       if (active) {
+       if (t == ERR_PTR(-EBUSY)) {
                taskq_wait_id(tq, id);
                rc = EBUSY;
        }
@@ -835,6 +838,7 @@ taskq_thread(void *args)
        taskq_ent_t *t;
        int seq_tasks = 0;
        unsigned long flags;
+       taskq_ent_t dup_task = {};
 
        ASSERT(tqt);
        ASSERT(tqt->tqt_tq);
@@ -894,22 +898,24 @@ taskq_thread(void *args)
                        list_del_init(&t->tqent_list);
 
                        /*
-                        * In order to support recursively dispatching a
-                        * preallocated taskq_ent_t, tqent_id must be
-                        * stored prior to executing tqent_func.
+                        * A TQENT_FLAG_PREALLOC task may be reused or freed
+                        * during the task function call. Store tqent_id and
+                        * tqent_flags here.
+                        *
+                        * Also use an on stack taskq_ent_t for tqt_task
+                        * assignment in this case. We only populate the two
+                        * fields used by the only user in taskq proc file.
                         */
                        tqt->tqt_id = t->tqent_id;
-                       tqt->tqt_task = t;
-
-                       /*
-                        * We must store a copy of the flags prior to
-                        * servicing the task (servicing a prealloc'd task
-                        * returns the ownership of the tqent back to
-                        * the caller of taskq_dispatch). Thus,
-                        * tqent_flags _may_ change within the call.
-                        */
                        tqt->tqt_flags = t->tqent_flags;
 
+                       if (t->tqent_flags & TQENT_FLAG_PREALLOC) {
+                               dup_task.tqent_func = t->tqent_func;
+                               dup_task.tqent_arg = t->tqent_arg;
+                               t = &dup_task;
+                       }
+                       tqt->tqt_task = t;
+
                        taskq_insert_in_order(tq, tqt);
                        tq->tq_nactive++;
                        spin_unlock_irqrestore(&tq->tq_lock, flags);
@@ -1238,10 +1244,18 @@ spl_taskq_init(void)
        if (system_taskq == NULL)
                return (1);
 
+       system_delay_taskq = taskq_create("spl_delay_taskq", MAX(boot_ncpus, 4),
+           maxclsyspri, boot_ncpus, INT_MAX, TASKQ_PREPOPULATE|TASKQ_DYNAMIC);
+       if (system_delay_taskq == NULL) {
+               taskq_destroy(system_taskq);
+               return (1);
+       }
+
        dynamic_taskq = taskq_create("spl_dynamic_taskq", 1,
            maxclsyspri, boot_ncpus, INT_MAX, TASKQ_PREPOPULATE);
        if (dynamic_taskq == NULL) {
                taskq_destroy(system_taskq);
+               taskq_destroy(system_delay_taskq);
                return (1);
        }
 
@@ -1261,6 +1275,9 @@ spl_taskq_fini(void)
        taskq_destroy(dynamic_taskq);
        dynamic_taskq = NULL;
 
+       taskq_destroy(system_delay_taskq);
+       system_delay_taskq = NULL;
+
        taskq_destroy(system_taskq);
        system_taskq = NULL;