Skip to content

Commit

Permalink
workqueue: Make sure struct worker is accessible for wq_worker_comm()
Browse files Browse the repository at this point in the history
The worker struct could already be freed when wq_worker_comm() tries
to access it for reporting.  This patch protects PF_WQ_WORKER
modifications with wq_pool_attach_mutex and makes wq_worker_comm()
test the flag before dereferencing worker from kthread_data(), which
ensures that it only dereferences when the worker struct is valid.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Lai Jiangshan <jiangshanlai@gmail.com>
Fixes: 6b59808 ("workqueue: Show the latest workqueue name in /proc/PID/{comm,stat,status}")
  • Loading branch information
Tejun Heo committed May 21, 2018
1 parent 6b59808 commit 197f6ac
Showing 1 changed file with 34 additions and 24 deletions.
58 changes: 34 additions & 24 deletions kernel/workqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,16 @@ static void process_scheduled_works(struct worker *worker)
}
}

static void set_pf_worker(bool val)
{
mutex_lock(&wq_pool_attach_mutex);
if (val)
current->flags |= PF_WQ_WORKER;
else
current->flags &= ~PF_WQ_WORKER;
mutex_unlock(&wq_pool_attach_mutex);
}

/**
* worker_thread - the worker thread function
* @__worker: self
Expand All @@ -2231,15 +2241,15 @@ static int worker_thread(void *__worker)
struct worker_pool *pool = worker->pool;

/* tell the scheduler that this is a workqueue worker */
worker->task->flags |= PF_WQ_WORKER;
set_pf_worker(true);
woke_up:
spin_lock_irq(&pool->lock);

/* am I supposed to die? */
if (unlikely(worker->flags & WORKER_DIE)) {
spin_unlock_irq(&pool->lock);
WARN_ON_ONCE(!list_empty(&worker->entry));
worker->task->flags &= ~PF_WQ_WORKER;
set_pf_worker(false);

set_task_comm(worker->task, "kworker/dying");
ida_simple_remove(&pool->worker_ida, worker->id);
Expand Down Expand Up @@ -2342,7 +2352,7 @@ static int rescuer_thread(void *__rescuer)
* Mark rescuer as worker too. As WORKER_PREP is never cleared, it
* doesn't participate in concurrency management.
*/
rescuer->task->flags |= PF_WQ_WORKER;
set_pf_worker(true);
repeat:
set_current_state(TASK_IDLE);

Expand Down Expand Up @@ -2434,7 +2444,7 @@ static int rescuer_thread(void *__rescuer)

if (should_stop) {
__set_current_state(TASK_RUNNING);
rescuer->task->flags &= ~PF_WQ_WORKER;
set_pf_worker(false);
return 0;
}

Expand Down Expand Up @@ -4580,37 +4590,37 @@ void show_workqueue_state(void)
/* used to show worker information through /proc/PID/{comm,stat,status} */
void wq_worker_comm(char *buf, size_t size, struct task_struct *task)
{
struct worker *worker;
struct worker_pool *pool;
int off;

/* always show the actual comm */
off = strscpy(buf, task->comm, size);
if (off < 0)
return;

/* stabilize worker pool association */
/* stabilize PF_WQ_WORKER and worker pool association */
mutex_lock(&wq_pool_attach_mutex);

worker = kthread_data(task);
pool = worker->pool;
if (task->flags & PF_WQ_WORKER) {
struct worker *worker = kthread_data(task);
struct worker_pool *pool = worker->pool;

if (pool) {
spin_lock_irq(&pool->lock);
/*
* ->desc tracks information (wq name or set_worker_desc())
* for the latest execution. If current, prepend '+',
* otherwise '-'.
*/
if (worker->desc[0] != '\0') {
if (worker->current_work)
scnprintf(buf + off, size - off, "+%s",
worker->desc);
else
scnprintf(buf + off, size - off, "-%s",
worker->desc);
if (pool) {
spin_lock_irq(&pool->lock);
/*
* ->desc tracks information (wq name or
* set_worker_desc()) for the latest execution. If
* current, prepend '+', otherwise '-'.
*/
if (worker->desc[0] != '\0') {
if (worker->current_work)
scnprintf(buf + off, size - off, "+%s",
worker->desc);
else
scnprintf(buf + off, size - off, "-%s",
worker->desc);
}
spin_unlock_irq(&pool->lock);
}
spin_unlock_irq(&pool->lock);
}

mutex_unlock(&wq_pool_attach_mutex);
Expand Down

0 comments on commit 197f6ac

Please sign in to comment.