myxce/.wolf/bugs/bug-1686.md
Rongsong Shen d26ff8870d
feat(buglog): one Markdown record per bug, checked against a record standard
The buglog was one 27K-line JSON file whose fields covered only symptom,
root cause and fix, as free text: 2% of entries named the code version a
bug was found on, 12% the build profile, 36% a way to reproduce, and 58% of
the file was OpenWolf hook auto-detections ("Missing guard clause",
"Function not marked async" on .md files).

- .claude/skills/buglog/bug-record.md: the record standard. Eight
  sections (Summary as the title, Description, Environment, Reproduction;
  Root cause, Fix, Verification, References once fixed), each with the
  content it must contain -- e.g. Environment needs the OS, the libc the
  failing binary loads (from its ELF interpreter: glibc 2.42 in the nix
  shell vs 2.40 on the host), toolchain, build profile and commit.
- .wolf/bugs/<id>.md: 500 records migrated from buglog.json; every field
  verified to round-trip. They carry legacy: true and an explicit "not
  recorded" where the old log had no such field; component derived from
  file paths, severity from keywords (else unknown), fix_commits from
  hashes in status_note/fix that git resolves (91 of 463 fixed).
- .wolf/buglog-archive.json: the 710 hook auto-detections, verbatim.
- .wolf/buglog.json is now an index generated from the records, keeping
  the fields OpenWolf's hooks and `openwolf bug search` read.
- scripts/buglog.py rewritten: new (template + Environment collected from
  the machine and --binary), check, status (refuses a status the record
  doesn't support; fixed needs Root cause, Fix, Verification and
  --commit hashes git resolves), note, recur, set, show, list, triage,
  env, index. 29 unittest cases in scripts/tests/test_buglog.py.
- scripts/git-hooks/pre-commit runs `check --staged`: incomplete staged
  records, a legacy record hand-changed to fixed without the fix-time
  sections, a new record claiming legacy: true, or a stale index refuse
  the commit.
- scripts/gitea-sync.py reads the records: issue body = the record's
  sections plus linked fix commits (origin remote, GITEA_COMMIT_REPO to
  override), and a body hash so an issue's text is refreshed when the
  record changes.
- bug-1866 rewritten to the full standard as the worked example.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-28 03:40:11 +00:00

8.3 KiB

id status status_updated status_note component severity timestamp last_seen occurrences files tags related legacy
bug-1686 fixed 2026-09-24 scheduler crash 2026-08-25T16:24:00Z 2026-09-19T09:00:00Z 1
src/scheduler/src/plugins/builtin_fcfs.c
scheduler
builtin_fcfs
crash
sigsegv
live-deployment
picker-plugin
bug-1685
true

bug-1686: User-reported live crash (build.dbg, commit b040dfc0 -- AFTER the bug-1685 double-free fix landed…

Description

User-reported live crash (build.dbg, commit b040dfc0 -- AFTER the bug-1685 double-free fix landed, so this is a distinct, separate crash): xce_scheduler SIGSEGV in fcfs_cursor_enter_queue (src/scheduler/src/plugins/builtin_fcfs.c:673), called from builtin_fcfs_next_job (builtin_fcfs.c:810) <- sched_job_picker (scheduler.c:118) <- picker thread. Crash line 673 is the NULL branch of a ternary expression. queue_item=0x7db7d0000000 -- a suspiciously round address (large power-of-2-aligned), possibly a corrupted/uninitialized pointer rather than a normal heap allocation. Core captured from a live 3-node HA deployment run (--peers ctrl-1/ctrl-2/ctrl-3, -vvv).

Environment

Not recorded (migrated from buglog.json).

Reproduction

Not recorded (migrated from buglog.json).

Root cause

FIXED (2026-08-25). fcfs_plugin_data_t.lock (added in the bug-1611-era concurrency fix, task 2.4 -- see builtin_fcfs.c's own file header Concurrency comment) correctly makes every individual hook call (on_queue_update/on_job_state_change/on_job_update/next_job/etc) mutually exclusive at the fcfs_plugin_data_t granularity -- but priv->cursor/priv->job_cursor are raw ordq_item_t* values that PERSIST in fcfs_plugin_data_t across SEPARATE builtin_fcfs_next_job() calls, each with its OWN independent lock acquisition/release. A properly-locked builtin_fcfs_on_queue_update() call for a queue name that already has a registered fcfs_queue_t re-sorts it by removing the queue's existing ordq_item_t node (ordq_get + FREEUP(it)) and inserting a brand-new one (ordq_put, fresh calloc). If priv->cursor is resting on that exact freed node -- which it normally is, since the cursor stays parked on a queue for as long as it keeps yielding ready jobs (sched_queue_is_ready()/sched_job_is_ready() are unconditional return 1 stubs today in scheduler.c, so a queue's cursor never advances away from it while jobs remain) -- the NEXT next_job() call dereferences priv->cursor->data from freed memory: a real heap-use-after-free, exactly matching this bug's captured live SIGSEGV in fcfs_cursor_enter_queue() (queue_item was a dangling, freed ordq_item_t*, not corrupted/uninitialized memory as originally suspected). The lock prevents CONCURRENT corruption within one call; it does nothing to stop a LATER call from dereferencing a node freed by a call that ran validly in between. The same shape exists for priv->job_cursor via builtin_fcfs_on_job_state_change()'s !JOB_IS_PEND removal branch and fcfs_remove_job_everywhere() (called by on_job_update() and on_dispatch()), both of which ordq_del() a job node job_cursor may be resting on.

Fix

FIXED (2026-08-25). Three invalidation sites in src/scheduler/src/plugins/builtin_fcfs.c, all following the same peek/compare-then-invalidate-before-free pattern:

  1. builtin_fcfs_on_queue_update()'s reuse branch: before FREEUP(it), if priv->cursor == it, sets both priv->cursor and priv->job_cursor to NULL (in lockstep, matching fcfs_cursor_enter_queue()'s own invariant that job_cursor is only ever meaningful relative to the queue cursor currently names).
  2. builtin_fcfs_on_job_state_change()'s !JOB_IS_PEND removal branch: ordq_peek()s the about-to-be-deleted job node first; if it equals priv->job_cursor, NULLs priv->job_cursor before ordq_del().
  3. fcfs_remove_job_everywhere() (shared by on_job_update() and on_dispatch()): same peek-then-invalidate-then-delete pattern inside its loop over all registered queues. Always safe: next_job() already treats a NULL cursor as "no more candidates this round" (pre-existing branch, unrelated to this bug -- see test_picker_fcfs_session.c's no-queues/empty-queue coverage), and begin_session() unconditionally resets both cursors fresh every round, so the worst case of an invalidation is a bounded one-round delay for the affected queue's jobs, never a crash or a permanently lost job.

Test: added src/scheduler/testsuite/test_builtin_fcfs_cursor_uaf.c, wired into src/scheduler/testsuite/meson.build (new sched_builtin_fcfs_cursor_uaf_test executable + 'scheduler builtin fcfs cursor uaf test' meson test, suite 'scheduler', ASAN_OPTIONS=detect_leaks=0, same convention as every sibling scheduler test). White-box #include of ../src/plugins/builtin_fcfs.c (same convention as test_picker_fcfs_session.c) for direct access to fcfs_plugin_data_t's private cursor/job_cursor fields; supplies its own always-ready sched_queue_is_ready()/sched_job_is_ready() test doubles and a plugin_register() stub to satisfy the linker (this TU does not link scheduler.c or plugin.c). The single test drives, on ONE thread with no real concurrency needed: begin_session() -> next_job() (dispatches job 1, leaves priv->cursor parked on queue "Q1"'s node with priv->job_cursor mid-walk) -> on_queue_update() again for the SAME queue name with a different priority (genuinely exercises the reuse-and-resort path, freeing the OLD ordq_item_t node priv->cursor is resting on) -> asserts priv->cursor/priv->job_cursor are now NULL (proves invalidation fired) -> next_job() again (the UAF reproduction point; post-fix returns NULL cleanly) -> a fresh begin_session()/next_job() round confirms job 1 and job 2 are BOTH still findable, proving no job is permanently lost, only delayed one round, matching the fix's documented safety property.

RED (pre-fix, verified live): temporarily reverted all three invalidation blocks in builtin_fcfs.c (kept the new test unchanged), rebuilt via nix develop ./src --command bash src/tools/xce-build.sh scheduler asan build-asan --incremental. First run: the test's own intermediate TEST_ASSERT_NULL(priv->cursor) check (step (e), asserting the invalidation fired) failed as a clean TEST_ASSERT failure -- Unity's assertion longjmp stopped the test function before it ever reached the dereferencing next_job() call, so the real UAF was not yet exercised. Temporarily disabled just that intermediate assertion (#if 0) to force execution through to the actual dereference and rebuilt: sched_builtin_fcfs_cursor_uaf_test aborted with a genuine ASAN heap-use-after-free -- 'AddressSanitizer: heap-use-after-free ... READ of size 8 ... #0 builtin_fcfs_next_job ../src/plugins/builtin_fcfs.c:813', freed-by stack 'FREEUP(it) <- builtin_fcfs_on_queue_update builtin_fcfs.c:452 <- test_cursor_invalidated_on_queue_resort_no_uaf test_builtin_fcfs_cursor_uaf.c:181', allocated-by stack 'ordq_put ../lib/ordq.c:48 <- builtin_fcfs_on_queue_update builtin_fcfs.c:457' -- an exact structural match to this bug's diagnosed mechanism and to its own captured live SIGSEGV in fcfs_cursor_enter_queue() <- builtin_fcfs_next_job() <- sched_job_picker(). Restored the temporarily-disabled assertion immediately after capturing this evidence. GREEN (post-fix, verified live): restored all three invalidation blocks in builtin_fcfs.c to their applied state (confirmed via git diff matching the pre-existing uncommitted fix exactly), rebuilt -- 'scheduler builtin fcfs cursor uaf test OK (0.06s)', 1 Tests 0 Failures. Full scheduler suite re-run via meson test --suite scheduler: 53/54 OK, 1 pre-existing failure (test_recordstart_symlink_escape_replies_error, bug-1560, unrelated -- 'Expected 'invalid record file name' Was 'permission denied accessing record file'', same failure present in the RED run too) -- zero new regressions. Note: a full top-level ninja build in this shared worktree also fails to link controller_unit_test (undefined references to test_persist_recovery/test_gc_retention/collect_confvars/etc.) due to unrelated, pre-existing uncommitted changes to src/controller/job.c/persist.c/meson.build from concurrent, unrelated work in this worktree (confirmed via git status: none of those files were touched by this fix) -- worked around by running meson test --suite scheduler directly against the already-built scheduler test binaries rather than requiring a full-tree ninja build to succeed.

Verification

Not recorded (migrated from buglog.json).

References

  • Related: bug-1685