⬢ DragonFlyBSD Kernel Audit
← triage · dashboard
DF-4533

drm_open() leaks one drm_device kref per successful open: the success-path drm_minor_acquire() is never balanced on any live path, so drm_dev_release()/drm_dev_fini() can never run and the whole drm_device leaks at unload

Summary

drm_open takes a minor ref and keeps it on success - Linux balances it in drm_release via file_priv->minor, but on DF the live release (drm_close) only balances its OWN close-time acquire; the alternative balance points (drm_release #ifdef, the unregistered dtor) are dead code. Net +1 drm_device kref per open/close cycle: dev->ref never returns to base so drm_dev_release never fires and the entire drm_device + driver-private memory leaks at unload (plus theoretical int-wrap after ~2^31 cycles). Save-and-release-the-open-minor fix.

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-4533 · 7 files
FileTypeDescriptionSize
trigger.c — 2.5 KB view raw
static_proof.sh — 1.2 KB view raw
static_proof.log — 2.0 KB view raw
fix.diff — 768 B view raw
fix_build.log — 1.2 KB view raw
VERDICT.md — 3.3 KB ↓ raw
verdict.json — 3.3 KB view raw
VERDICT.md
↓ download raw

DF-4533 VERDICT

Classification: REAL BUG, inconclusive at runtime (hardware-gated), source-certain.

Root cause chain

  1. sys/dev/drm/drm_file.c:325 — drm_open() acquires the minor (drm_minor_acquire() → drm_dev_get(), drm_drv.c:271-290).
  2. drm_file.c:343-358 — on failure, err_undo drops it (:357). On success the function returns with the reference deliberately held — this mirrors Linux, where the file owns the reference and drm_release() (the fops ->release) drops it via file_priv->minor.
  3. On DragonFly the release fop equivalent is drm_close(): - drm_file.c:373 — acquires a fresh minor reference (this is the DF-1678-era safety acquire), - drm_file.c:445 — releases that fresh reference. The open-held reference from step 2 is never released: net +1 dev kref per open/close cycle.
  4. The two other would-be balances are dead: - drm_release() (drm_file.c:595-604) returns 0 immediately on __DragonFly__ before any release; - drm_cdevpriv_dtor() (drm_drv.c:1194-1236) is never registered (no devfs_set_cdevpriv() caller in sys/dev/drm — mechanically proven by static_proof.sh).

Consequence

  • dev->ref grows without bound; drm_dev_release() (drm_drv.c:737-747) never fires, so at driver unload the entire struct drm_device (and the driver-private memory hanging off it) leaks instead of being finalized by drm_dev_fini() (drm_drv.c:666-692).
  • Any video-group user can grow the count by cycling open()/close() (trigger.c); ~2^31 cycles would wrap the int-based atomic counter (theoretical, hours of continuous cycling).

Severity: Low — bounded real-world impact (one-time dev leak at unload + refcount growth); no memory-safety consequence demonstrated. It sits squarely in the DF-1678 family ("drm_open/release vs unload — the minor refcount protocol"), which pass 2 was asked to extend.

What was run

Step Result
static_proof.sh on the repo PASS — all four acquire/release sites enumerated; drm_release() dead on DFly; dtor unregistered
trigger.c built on guest (-Wall -Wextra -O2) clean, RC=0
trigger 1000 on guest hardware-gated: /dev/dri/card0: No such file or directory
fix.diff applies to pristine sys/dev/drm/drm_file.c verified (patch --dry-run + apply on scratch copy)
patched drm_file.c compiled with exact kernel CFLAGS RC=0 (fix_build.log, object /tmp/drm_file_4533.o)
guest source restored md5 matches repo

Fix

fix.diff: save file_priv->minor into open_minor at drm_close() entry (before the kfree at :430) and drm_minor_release(open_minor) at the tail, next to the existing release of the close's own acquire. Keeps the DF-1678 safety acquire intact while restoring the Linux refcount pairing. Same-anchor note: DF-4532's fix.diff anchors at the same tail — merge by hand if both are applied.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.

Fix verification

not_testable
baseline no→ patch + rebuild →patched clean

Patch compiles clean (RC=0) with exact X86_64_GENERIC kernel CFLAGS on the guest; applies cleanly to the pristine tree. Runtime validation needs a DRM device (absent here); the fix restores the Linux acquire/release pairing by construction while keeping the DF-1678-era close-time safety acquire.

findings/poc/DF-4533/fix.diff; findings/poc/DF-4533/fix_build.log
↓ fix.diffper-fix-DF-4533

Confirmed kernel references

Detail

Evidence (decisive lines)

static_proof.log (PASS); VERDICT.md (full acquire/release ledger); fix.diff (applies clean); fix_build.log (patched drm_file.c compiles RC=0 with exact kernel CFLAGS); run evidence in DF-4531/run.log (hardware-gated trigger output on the shared guest)

PoC changes

Wrote from scratch: trigger.c (open/close cycle driver) and static_proof.sh (tree-level proof of the imbalance).

Verified recommended fix

In drm_close(), save file_priv->minor before kfree(file_priv) and drm_minor_release() it at the tail (balancing drm_open's success-path acquire) - see fix.diff.

Verdict

REAL BUG, runtime-gated on absent hardware. drm_open() takes a minor reference at drm_file.c:325 (drm_minor_acquire -> drm_dev_get) and, mirroring Linux, keeps it on SUCCESS - the open file owns it and Linux's drm_release() drops it via file_priv->minor. On DragonFly the live release path is drm_close(), which releases ONLY its own fresh acquire (drm_file.c:373 acquire / :445 release); the open-held reference is never dropped anywhere: drm_release() is dead on DragonFly (returns at :604) and drm_cdevpriv_dtor() (drm_drv.c:1194) is never registered (no devfs_set_cdevpriv caller - mechanically proven). Net +1 drm_device kref per open/close cycle: dev->ref never returns to base, drm_dev_release()/drm_dev_fini() (drm_drv.c:737/666) can never run, so the whole drm_device leaks at driver unload, and the unbounded int atomic counter can in principle be wrapped by ~2^31 cycles (trigger.c). Low severity (bounded leak, no memory-safety consequence); extends the DF-1678 'open/release vs unload minor refcount protocol' family as requested by pass 2.