β¬’ DragonFlyBSD Kernel Audit
← triage Β· dashboard
DF-5595

sim refcount release protocol is non-atomic and unenforced β€” asymmetric with its atomic acquisition side and its cmpset-loop neighbors

Summary

xpt_bus_register acquires with atomic_add_int while cam_sim_release releases with a plain read-compare-decrement/free under no lock: two unsynchronized releases lose updates (leak or double kfree + double simq release), and a release racing an acquisition outside the sim lock can free early. Hardening class (in-tree sites serialized by driver convention only); the cmpset rewrite is implemented and validated inside DF-5592's fix.diff.

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-5595 Β· 4 files
FileTypeDescriptionSize
README.md β€” 1.8 KB ↓ raw
verdict.json β€” 2.0 KB view raw
env.txt β€” 515 B view raw
manifest.json β€” 560 B view raw

DF-5595 β€” sim refcount release protocol is non-atomic and unenforced (asymmetric with its acquisition side)

sys/bus/cam/cam_sim.c pass-2 β€” cam_sim_release().

What

  • Acquisition: xpt_bus_register() takes a sim reference with atomic_add_int(&sim->refcount, 1) (cam_xpt.c:4322) under the sim lock + topo lock.
  • Release: cam_sim_release() (cam_sim.c:246-255) does a plain if (sim->refcount == 1) { ... kfree ... } else { --sim->refcount; } β€” a non-atomic read-compare-decrement/free with no lock requirement checked or documented anywhere in cam_sim.c.
  • Every sibling finalizer in the same subsystem uses atomic cmpset loops precisely because these objects die concurrently: xpt_release_bus (cam_xpt.c:4979-5008), xpt_release_target (5048-5070), xpt_release_device (5192-5232). cam_sim_release() is the lone pre-modernization holdout (and DF-5552 documented the same defect pattern one level down, on the devq).

Two unlocked/unserialized cam_sim_release() calls racing (or a release racing an acquisition outside the sim lock) lose updates: both read the same count, both write count-1 β†’ reference leak (sim never freed, devq never released), or both read 1 β†’ double kfree(sim) + double cam_simq_release(devq). In-tree call sites (xpt_bus_deregister at cam_xpt.c:4484, driver cam_sim_free) are serialized only by per-driver lock convention β€” nothing in cam_sim.c enforces or documents it.

Status

Hardening-class: no in-tree unsynchronized release pair demonstrated (same standing as DF-5552, its devq sibling). The fix is subsumed by DF-5592's validated fix.diff, which rewrites cam_sim_release() into the atomic cmpset protocol mirroring xpt_release_bus() β€” one more reason that fix is the right shape. Filed Low / likely; PoC untested.

Fix verification

not_testable
baseline no→ patch + rebuild →patched clean

not separately validated; the rewrite ships inside DF-5592's validated fix.diff

DF-5592/fix.diff; DF-5592/fix_run.log
per-fix-DF-5595

Confirmed kernel references

Detail

Evidence (decisive lines)

README.md (code citations); DF-5592 fix.diff (the validated rewrite of cam_sim_release)

Verified recommended fix

Atomic cmpset release protocol in cam_sim_release() (as implemented and validated in DF-5592's fix.diff)

Verdict

Hardening-class, trace-verified. cam_sim_release() (cam_sim.c:246-255) decrements/frees the sim with a plain non-atomic read-compare-decrement and no lock enforcement or documentation, while the acquisition side xpt_bus_register() uses atomic_add_int (cam_xpt.c:4322) and every sibling finalizer (xpt_release_bus 4979-5008, xpt_release_target 5048-5070, xpt_release_device 5192-5232) uses atomic cmpset loops. Unserialized release pairs lose updates (ref leak: sim+devq never freed; or double kfree + double cam_simq_release). In-tree call sites are sim-lock serialized only by per-driver convention. Same class as DF-5552 (devq), one level up. The concrete rewrite is included and validated inside DF-5592's fix.diff (cam_sim_release becomes the atomic cmpset protocol), which is the recommended fix.