DF-2788 — callout_stop/cancel/drain/stop_async return 0 after dequeuing a pending callout

What

_callout_cancel_or_stop() (sys/kern/kern_timeout.c:859-930) processes a STOP/CANCEL request synchronously via _callout_update_spinlocked() (:881). When the callout was queued (CALLOUT_SET) and not executing, the update dequeues it from the callwheel and sets CALLOUT_PREVENTED on the backend _callout (kern_timeout.c:229 for CANCEL, :276 for STOP). Immediately after, the fast path at kern_timeout.c:888-891

if (sync == 0 || (c->flags & (CALLOUT_INPROG | CALLOUT_SET)) == 0) {
        exis_drop_gd(gd);
        return 0;                      /* <-- unconditionally */
}

sees (INPROG|SET) == 0 (we just cleared SET by dequeuing) and returns 0, never reading the PREVENTED bit it just caused to be set. The near-identical recheck a few lines later (:895-901) shows the intended semantics — return ((c->flags & CALLOUT_PREVENTED) != 0) — but is only reachable if the flags changed between :888 and :895 (a race window a few instructions wide), i.e. it is effectively dead code.

Consequently, on a stock kernel callout_stop(), callout_stop_async(), callout_cancel() and callout_drain() return 0 for the common case "this call removed a pending callout from the wheel", violating the documented contract at kern_timeout.c:1052-1058, :1027-1030, :1080-1094 ("Returns whether the STOP operation was responsible for removing a queued or pending callout" / "Returns 1 if the cancel/drain is responsible for stopping the callout").

This is the second half of the broken-return family; DF-0048 (known, not re-reported here) is the other half: on the INPROG-completion path the handler records PREVENTED on c->verifier->flags (the frontend struct callout, :600-601, :623-624) while the reader looks at the backend c->flags. Fixing DF-0048 alone does not restore correct returns — the :888 fast path still swallows the dequeue case fixed by this finding's patch (proven by the patched-kernel run below, which fixes only this half and flips all four returns to 1).

Impact

Correctness / API-contract, Low. Return-value-checking callers take the wrong branch. In-tree example: sys/netgraph/pptpgre/ng_pptpgre.c:786/889 if (callout_stop(&a->rackTimer)) { kfree(a->rackTimerPtr, ...); ng_unref(node); } — the free/unref arm is dead code on a stock kernel (reference + memory leak while timers were armed; the timeout callback still frees its own arg, so this direction is a leak, not a UAF). No false "1" is possible: PREVENTED-on-backend is only set at dequeue time under both c->spin and wheel->spin with INPROG clear, so a return of 1 can never coincide with a queued or executing callback — no firing-after-free primitive exists in either half of the family.

Reproduce (guest: DragonFly 6.5-DEVELOPMENT, X86_64_GENERIC, INVARIANTS)

scp df2788.c Makefile build.sh run.sh dfbsd:/root/df2788/
ssh dfbsd: cd /root/df2788 && sh build.sh    # KLD, -Werror clean
ssh dfbsd: cd /root/df2788 && sh run.sh      # kldload + sysctl -n debug.df2788=1

Expected (stock kernel, bug): dmesg

DF2788 A stop:      pending 1 -> 0, ret=0 (contract: 1)
DF2788 B stop_async: pending 1 -> 0, ret=0 (contract: 1)
DF2788 C cancel:    pending 1 -> 0, ret=0 (contract: 1)
DF2788 D drain:     pending 1 -> 0, ret=0 (contract: 1)
DF2788 E control:   fired=1, ret=0 (contract: 0)
DF2788 RESULT: stop=0 async=0 cancel=0 drain=0 ctl_fired=1 ctl_ret=0 BUG=1

(pending flips 1 -> 0 proves this call dequeued the callout; ret=0 is the contract violation; control case E shows 0 is the correct answer only when nothing was pending.)

Deterministic — no race, no special setup, root only (KLD).

Fix validation

fix.diff (git-apply clean against sys/): the :888 fast path returns PREVENTED from the backend for both sync and async when the request was already processed (PREVENTED was cleared at :879, so it cannot be stale); the async still-pending case keeps returning 0.

• Baseline (kernel #0, stock): BUG=1 (run.log, run.2.log)
• Patched (kernel #1, make -j6 nativekernel KERNCONF=X86_64_GENERIC + installkernel + reboot, fix_build.log): BUG=0, all four ret=1 with pending 1->0, control ret=0 (run_patched.log)

Files

df2788.c, Makefile — trigger module; build.sh / run.sh — exact commands; build.log — full clean KLD build (-Werror, RC=0); run.log / run.2.log — stock kernel runs; fix.diff — the fix; fix_build.log — patched kernel build; run_patched.log — patched kernel re-run of the identical PoC.

DF-2788 VERDICT — reproduced (correctness bug, Low); fix validated in-guest

Verdict

REPRODUCED, deterministic, root-only (KLD trigger). On the stock INVARIANTS guest kernel (#0, Thu Jul 2 06:02:54 UTC 2026), for each of callout_stop(), callout_stop_async(), callout_cancel(), callout_drain(): arming an mpsafe callout hz ticks out, observing callout_pending()==1, invoking the API, and observing callout_pending()==0 — proving this call dequeued the callout — the API returned 0 every time, where the documented contract requires 1 ("responsible for removing a queued or pending callout"). The control case (callout fires, then stop) correctly returns 0, so the return value cannot distinguish "I removed it" from "nothing to remove". Impact is correctness/leak-side only (e.g. ng_pptpgre.c:786/889 free-arm is dead code); no memory-safety primitive — a return of 1 can never coincide with a queued or executing callback because PREVENTED-on-backend is set only at dequeue time under c->spin + wheel->spin with INPROG clear (kern_timeout.c:218-231, :266-278), so neither half of the broken-return family (this + DF-0048) yields firing-after-free.

Root cause (path:line)

• _callout_cancel_or_stop() clears PREVENTED, sets the request flag, and processes it synchronously: sys/kern/kern_timeout.c:877-882.
• The update dequeues a queued callout and sets CALLOUT_PREVENTED on the backend c: sys/kern/kern_timeout.c:225-231 (CANCEL), :273-278 (STOP).
• The fast path at sys/kern/kern_timeout.c:888-891 sees (c->flags & (CALLOUT_INPROG|CALLOUT_SET)) == 0 (SET was just cleared by our own dequeue) and return 0; unconditionally — the PREVENTED bit is never read.
• The intended read exists but is ~dead code: sys/kern/kern_timeout.c:895-901 (return ((c->flags & CALLOUT_PREVENTED) != 0);) is reachable only if the flags changed in the few-instruction window between :888 and :895.
• Distinct root cause from DF-0048 (known, not re-reported): DF-0048 is the INPROG-completion path setting PREVENTED on c->verifier->flags (frontend) at :600-601/:623-624 while the reader reads the backend. This finding is the dequeue path setting PREVENTED correctly on the backend but the reader returning early. Fixing DF-0048 alone leaves the bug (this PoC's cases A-D do not involve INPROG at all); fixing this finding alone leaves DF-0048's INPROG-race case wrong. Both need fixing for a truthful return value.

Evidence

• run.log / run.2.log (stock kernel #0, two consecutive runs): DF2788 RESULT: stop=0 async=0 cancel=0 drain=0 ctl_fired=1 ctl_ret=0 BUG=1 with all four pending 1 -> 0 transitions (full marker lines captured via dmesg in run.2.log).
• run_patched.log (kernel #1 with only fix.diff applied): DF2788 RESULT: stop=1 async=1 cancel=1 drain=1 ctl_fired=1 ctl_ret=0 BUG=0 — the identical PoC returns the documented contract values.
• fix_build.log: make -j6 nativekernel KERNCONF=X86_64_GENERIC + make installkernel, KERNEL_RC=0 / INSTALL_RC=0.
• build.log: full KLD build, -Werror clean.

Exploit chain

None (impact=none). Not memory corruption: the wrong return is "0 instead of 1" — callers that free on 1 never free on the wrong path (leak, not UAF), and no path returns 1 while the callback is queued or running (proven by the flag-state analysis above). Escalation to uid=0 is not applicable to this class.

Why the rest of the audit is a negative result (pass-2 depth)

Beyond this finding and the known DF-0048, every candidate traced during the pass-2 audit was killed on the source; the notable ones:

• Lost-cancel / fire-after-stop: impossible — the sync wait loop (kern_timeout.c:911-921) sleeps under c->spin (ssleep releases/reacquires atomically, kern_synch.c:814-828) and the request flags are cleared only inside _callout_update_spinlocked terminal branches under c->spin with wakeup (kern_timeout.c:225-231, :273-278, :300-304, :341-343); the handler skips dispatch when STOP|CANCEL is observed after INPROG is set (:590-602).
• Cross-cpu drain hang: drain sleeps (no spinning); a hang requires a callback that blocks forever — caller defect, not this file.
• Lock-order: global order is c->spin -> wheel->spin everywhere (:205, :243-265, :519-570, :643-649); the handler never holds both.
• Reset-while-executing (self-reinit, RFC pattern) and reset-migration to another cpu: serialized by c->spin; INPROG blocks third-party updates (:184-192) and the recursive same-thread case is explicitly allowed; insertion into the target slot is race-free against softtick advance because the inserter holds the target wheel->spin across the clamp-check + TAILQ_INSERT (:257-265, :322-333) and softticks can only reach qtick by scanning that slot under the same lock.
• hardclock_softtick vs softclock_handler (isrunning/softticks handoff): isrunning is set/cleared only in crit-serialized same-cpu contexts (:455-479, :682-688); the handler's softticks++ at :652 is covered by isrunning==1 for its whole run. No lost-wakeup interleaving exists.
• tsleep quick-callout protocol (kern_synch.c:690-760, endtsleep :935-982): TDF_TIMEOUT_RUNNING is set under crit before any blocking, the tsleep side spins until it clears, and _callout_cancel_quick's KKASSERT(INPROG==0) cannot fire because same-cpu crit excludes the softclock thread. Airtight.
• Freelist/EXIS discipline: _callout_free (:356-368) only runs after callout_terminate's synchronous cancel flushed all spin-holders; the kfree at :664 is gated on exis_freeable (refs taken by _callout_gettoc :719 before reading cc->toc freeze pseudo_ticks, kern_clock.c:797-813), so no use-after-free of a backend _callout is constructible without a caller racing callout_reset against callout_terminate (illegal caller discipline; e.g. the verifier check at :547/:539 catches most such misuse with a panic).
• Tick arithmetic: curticks/softticks/rtick are consistently mod-2^32; extreme to_ticks (near INT_MAX) wraps the clamp subtraction into a 1-tick clamp (early fire, bounded) — same family as the DF-2688 negative-to-1-tick clamp already on record as consumer context; not a defect here.
• callout_reset_bycpu(cpuid) has no bounds check on the array index, but every in-tree caller passes kernel-chosen cpuids (constants, mycpuid, driver-computed); KLD-only misuse. Hardening note only.