c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-11-17 18:11:00 +03:00
Code Issues Packages Projects Releases Wiki Activity

Store copy of tqent_flags prior to servicing task

Browse Source 
A preallocated taskq_ent_t's tqent_flags must be checked prior to
servicing the taskq_ent_t. Once a preallocated taskq entry is serviced,
the ownership of the entry is handed back to the caller of
taskq_dispatch, thus the entry's contents can potentially be mangled.

In particular, this is a problem in the case where a preallocated taskq
entry is serviced, and the caller clears it's tqent_flags field. Thus,
when the function returns and task_done is called, it looks as though
the entry is **not** a preallocated task (when in fact it **is** a
preallocated task).

In this situation, task_done will place the preallocated taskq_ent_t
structure onto the taskq_t's free list. This is a **huge** mistake. If
the taskq_ent_t is then freed by the caller of taskq_dispatch, the
taskq_t's free list will hold a pointer to garbage data. Even worse, if
nothing has over written the freed memory before the pointer is
dereferenced, it may still look as though it points to a valid list_head
belonging to a taskq_ent_t structure.

Thus, the task entry's flags are now copied prior to servicing the task.
This copy is then checked to see if it is a preallocated task, and
determine if the entry needs to be passed down to the task_done
function.

Signed-off-by: Prakash Surya <surya1@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #71
This commit is contained in:
Prakash Surya 2011-12-16 14:57:31 -08:00 committed by Brian Behlendorf
parent e7e5f78e7b
commit 8f2503e0af
2 changed files with 17 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/sys/taskq.h
View File

@ -97,6 +97,7 @@ typedef struct taskq_thread {
struct task_struct *tqt_thread;
taskq_t *tqt_tq;
taskqid_t tqt_id;
uintptr_t tqt_flags;
} taskq_thread_t;
/* Global system-wide dynamic task queue available for all consumers */

22
module/spl/spl-taskq.c
View File

@ -135,11 +135,6 @@ task_done(taskq_t *tq, taskq_ent_t *t)
ASSERT(t);
ASSERT(spin_is_locked(&tq->tq_lock));
/* For prealloc'd tasks, we don't free anything. */
if ((!(tq->tq_flags & TASKQ_DYNAMIC)) &&
(t->tqent_flags & TQENT_FLAG_PREALLOC))
return;
list_del_init(&t->tqent_list);
if (tq->tq_nalloc <= tq->tq_minalloc) {
@ -147,6 +142,7 @@ task_done(taskq_t *tq, taskq_ent_t *t)
t->tqent_func = NULL;
t->tqent_arg = NULL;
t->tqent_flags = 0;
list_add_tail(&t->tqent_list, &tq->tq_free_list);
} else {
task_free(tq, t);
@ -480,10 +476,19 @@ taskq_thread(void *args)
if (pend_list) {
t = list_entry(pend_list->next, taskq_ent_t, tqent_list);
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. */
tqt->tqt_id = t->tqent_id;
/* 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;
taskq_insert_in_order(tq, tqt);
tq->tq_nactive++;
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -494,7 +499,11 @@ taskq_thread(void *args)
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags);
tq->tq_nactive--;
list_del_init(&tqt->tqt_active_list);
task_done(tq, t);
/* For prealloc'd tasks, we don't free anything. */
if ((tq->tq_flags & TASKQ_DYNAMIC) ||
!(tqt->tqt_flags & TQENT_FLAG_PREALLOC))
task_done(tq, t);
/* When the current lowest outstanding taskqid is
* done calculate the new lowest outstanding id */
@ -504,6 +513,7 @@ taskq_thread(void *args)
}
tqt->tqt_id = 0;
tqt->tqt_flags = 0;
wake_up_all(&tq->tq_wait_waitq);
}