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

Lost-wakeup race in ngdread: NGDF_RWAIT armed after the empty-queue check

Summary

ngdread checks IF_DEQUEUE(&priv->readq) BEFORE taking ngd_mtx and setting NGDF_RWAIT; the producer enqueues and only wakes if the flag is already set. An enqueue landing between the two loses its wakeup, so read(2) blocks with data queued until the next packet or a signal (PCATCH rescues). Reader-side stall only, no memory-safety impact; latent in dead code. Phase V skipped (dead code, Low).

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-3548 Β· 2 files
FileTypeDescriptionSize
README.md file 1.6 KB ↓ raw
verdict.json file 1.6 KB view raw
README.md file
↓ download raw

DF-3548 β€” lost-wakeup race in ngdread (ng7 ng_device)

ngdread checks the queue BEFORE arming NGDF_RWAIT: * sys/netgraph7/ng_device.c:419 IF_DEQUEUE(&priv->readq, m) (queue empty β†’ m==NULL) * sys/netgraph7/ng_device.c:423-425 only then mtx_lock; flags |= NGDF_RWAIT; msleep(...)

while the producer arms the wakeup only if the flag is already set: * sys/netgraph7/ng_device.c:277 _IF_ENQUEUE(&priv->readq, m) * sys/netgraph7/ng_device.c:279-284 mtx_lock; if (flags & NGDF_RWAIT) { clear; wakeup(priv); }

If _IF_ENQUEUE + flag-check land between :419 and :423, the wakeup is lost and the reader sleeps despite a queued packet (until the NEXT enqueue or a signal β€” PCATCH). Reader-side stall only; no memory-safety impact.

Phase V: skipped (Low severity hardening-class finding; also unbuildable dead code β€” same rot as DF-3547, see ../DF-3547/build.log).

Fix sketch (in the intended FreeBSD semantics of the file):

        do {
            IF_DEQUEUE(&priv->readq, m);
            if (m == NULL) {
                if (flag & IO_NDELAY)
                    return (EWOULDBLOCK);
                mtx_lock(&priv->ngd_mtx);
                priv->flags |= NGDF_RWAIT;
-               if ((error = msleep(priv, &priv->ngd_mtx,
+               _IF_DEQUEUE-check again under ngd_mtx before sleeping, or
+               set NGDF_RWAIT under the readq lock / re-check the queue
+               atomically with setting the flag:
+               IF_LOCK(&priv->readq);
+               if (!IFQ_IS_EMPTY(&priv->readq)) { IF_UNLOCK; mtx_unlock; continue; }
+               IF_UNLOCK;
+               if ((error = msleep(priv, &priv->ngd_mtx,
                    PDROP | PCATCH | (PZERO + 1), "ngdread", 0)) != 0)
                    return (error);

Fix verification

not_testable
baseline no→ patch + rebuild →patched clean

not_applicable β€” skipped Low finding in non-compilable dead code

n/a
per-fix-DF-3548

Confirmed kernel references

Detail

Evidence (decisive lines)

README.md path:line chain; ../DF-3547/build.log (file rot)

PoC changes

no PoC authored (skipped)

Verified recommended fix

Re-check the queue atomically with setting NGDF_RWAIT (hold the readq lock across check+flag, or re-dequeue under ngd_mtx before msleep) β€” see README.md sketch.

Verdict

SKIPPED per contract (Low-severity correctness/hardening finding). Certain by source reading: ngdread checks IF_DEQUEUE (ng_device.c:419) before arming NGDF_RWAIT (:423-425), so a producer enqueueing at :277 and checking the flag at :280 in that window never wakes the reader β€” read(2) stalls until the next packet or a signal. Reader-side stall only, no memory-safety impact; additionally the file is unbuildable dead code (see DF-3547), so no runtime verification is possible or meaningful.