From da06ba437f1141864e896204f8aa07af20440be2 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 30 Jun 2026 14:27:59 +0000 Subject: [PATCH] docs(raft-ha-core-safety): 5.8 validated (bug-1298 UAF fixed via ep refcount); round-4..6 reveals pre-existing commit-loss (task 5.9) 5.8 endpoint refcount fix validated: 182 unit tests/0 fail, 6-round ASAN chaotic soak ASan-clean, all rounds elect. bug-1298 marked fixed. The soak now reaches rounds 4-6 for the first time (every prior run crashed at round 4) and FAILs the verdict on committed_job_loss=4/divergence=7. Evidence shows this is the pre-existing bug-795/828/851 failover catch-up gap, NOT a regression: one job missing from round 4 onward across 3 primaries; rounds 1-3 clean; ZERO WAL/commit/catch-up code changed this session; ASAN clean. Routed to task 5.9 / ha-commit-durability. --- .kiro/specs/raft-ha-core-safety/research.md | 110 ++++++++++++++++++++ .kiro/specs/raft-ha-core-safety/tasks.md | 20 ++-- .wolf/buglog.json | 103 +++++++++++++++++- 3 files changed, 224 insertions(+), 9 deletions(-) diff --git a/.kiro/specs/raft-ha-core-safety/research.md b/.kiro/specs/raft-ha-core-safety/research.md index 62354582..a705de82 100644 --- a/.kiro/specs/raft-ha-core-safety/research.md +++ b/.kiro/specs/raft-ha-core-safety/research.md @@ -719,3 +719,113 @@ latent UAF that the bug-1282 lockless `ep->busy` read unmasked (the dispatcher n reaches the read without contending on `ep->lock`). Logged as **bug-1298**; tracked as task 5.8. Consensus safety (Phase-2/5.2/5.3/5.5/5.6/5.7) is sound; elections and job safety hold for every round that completes. + +--- + +# Cross-Spec Gap Analysis (2026-06-30) + +Generated by `/kiro-validate-gap ha-term-safety raft-drive-async raft-ha-core-safety ha-consistency-invariants`. + +## Summary + +4-spec HA safety family: **ha-term-safety COMPLETE**, **raft-drive-async DORMANT** (all tasks done, production flags OFF), **raft-ha-core-safety IN PROGRESS** (12/26 tasks, Phase 5 critical path — task 5.7), **ha-consistency-invariants DESIGN ONLY**. One blocker to production: raft-ha-core-safety 7.2 chaotic soak PASS. + +## Dependency Graph + +``` +ha-consistency-invariants (umbrella — defines invariants) + ├── ha-term-safety (term recording + reconcile) ← COMPLETE + ├── raft-ha-core-safety (payload safety + election liveness + soak gate) ← IN PROGRESS + └── raft-drive-async (non-blocking drive threads) ← DORMANT + gates on: raft-ha-core-safety 7.2 PASS +``` + +## Implementation State + +| Spec | Tasks Done | Tasks Remaining | Status | +|------|-----------|----------------|--------| +| ha-term-safety | 20/20 | 0 | COMPLETE (build green, 175 tests, clean failover verified) | +| raft-ha-core-safety | 12/26 | 14 (5.7 critical) | IN PROGRESS (Phases 1-2+4+5.1-5.4 done; Phase 3 not started) | +| raft-drive-async | 17/17 | 0 (dormant flags) | DORMANT (production flags OFF; gates on raft-ha-core-safety 7.2) | +| ha-consistency-invariants | 0/0 | design only | DESIGN ONLY (no tasks generated) | + +## Critical Path + +``` +raft-ha-core-safety 5.7 (vote-reply term-lag) → 7.2 (soak PASS) → raft-drive-async flags ON +``` + +## Remaining Gaps + +1. **Task 5.7** (NOT STARTED): vote-reply term-lag — candidate's term escalates faster than grant RTT, all grants arrive stale. Fix: don't bump election term while vote round-trip outstanding, or warm peer connections. +2. **Task 7.2** (BLOCKED on 5.7): acceptance chaotic soak gate — ≥6 rounds, zero divergence, zero job loss. Currently: rounds 1-6 pass, round 7 stalls on PreVote cross-denial (bug-1296). Election liveness is the LAST remaining data-path gap; payload safety (Phase 2) and reconvergence (Phase 4) are proven. +3. **Phase 3** (snapshot-boundary reconcile): lowest priority — snapshot install is rare (16-byte sentinel today). +4. **raft-drive-async flags**: peerctrl/snapshot/configpush all default OFF. Flag-flip gate = raft-ha-core-safety 7.2 PASS + borrowed-ep refcount for snapshot/configpush lane. +5. **ha-consistency-invariants**: umbrella spec with approved design, no tasks. Close by verifying all invariants are satisfied by the three implementation specs. + +## Risk + +| Risk | Likelihood | Impact | +|------|-----------|--------| +| Task 5.7 reveals further election liveness layers (5.5→5.6→5.7 chain) | HIGH | Schedule slip; budget 2-3 more iterations | +| Chaotic soak still fails after 5.7 | MEDIUM | 7.2 blocks raft-drive-async; consider sequential-only fallback | +| Raft-drive-async flag-flip reveals new races | LOW | Gradual rollout: peerctrl first | + +## Recommendations + +1. **Immediate**: Fix task 5.7 (vote-reply term-lag) — approach (a): don't bump term while vote outstanding. +2. **Short-term**: Complete raft-ha-core-safety 5.7→7.2→7.3, then flip raft-drive-async flags. +3. **Medium**: Generate tasks from ha-consistency-invariants design; close the umbrella loop. + +--- + +## Task 5.8 fix VALIDATED + round-4..6 reveals pre-existing commit-loss (2026-06-30) + +**5.8 (endpoint teardown-vs-dispatch UAF, bug-1298) — FIXED, commit 6e05dc1.** + +Chosen fix (user picked the principled option over the targeted deferred-teardown +patch): an endpoint REFERENCE COUNT consolidating the prior ad-hoc lifetime +patches. endpoint_t.refcnt (_Atomic) + heap_alloc; endpoint_get → refcnt=1 (owner); +vtask_enq takes a per-vtask ref BEFORE publishing the task to the dispatcher, +run_task / taskq_item_killer release it; every heap-ep teardown drops its owner ref +via endpoint_unref (last unref fini+frees). Embedded eps (client ctx->ep, +heap_alloc=0) are never freed by unref. So the dispatcher reads ep->busy on a queued +vtask without the io_worker freeing it underneath — the UAF is structurally +impossible. Also fixed several latent ep-struct leaks (endpoint_fini-only sites). + +Validation: controller_unit_test 182 tests / 0 failures (test_ha_vep_teardown +OFF-path test updated — it asserted the now-fixed struct leak). 6-round ASAN chaotic +soak: ZERO AddressSanitizer reports, ctrl-3 survives round 4, ALL 6 rounds elect at +bounded terms (5→6→7→8). The round-4 crash-stall is gone. + +**The soak now reaches rounds 4-6 for the FIRST TIME — and reveals a pre-existing +committed-job-loss (task 5.9, NOT a regression).** + +Verdict: committed_job_loss=4, committed_divergence=7 → FAIL. But this is NOT a +regression of 5.8 nor of any previously-fixed bug. Proof: + +1. ONE specific committed job (0-0-1782829181-1) goes missing at round 4 and stays + missing through rounds 5 and 6, across THREE different elected primaries + (ctrl-3, ctrl-2, ctrl-3). A permanent committed-prefix gap that propagates as + catch-up streams FROM the gapped node — the exact bug-795/828/851 signature. +2. Rounds 1-3 are CLEAN (all jobs present, zero divergence). Loss begins precisely + at round 4 — the first round whose overlapping kills elect a JUST-RESTARTED node + (ctrl-2 restarted, ctrl-1 killed, ctrl-3 — itself recently restarted — elected). + A protocol/catch-up condition, not a memory condition. +3. This session changed ZERO WAL/commit/catch-up/replication code (persist.c, + config_repl.c, commit.c, repl*). The files touched are cluster.c, ha.c/.h, + netio.c, vtaskmgr.c (ep refcount + the 5.7 election timeout) + 2 test files. The + commit-durability path is byte-identical to the session start — it CANNOT have + regressed because it did not change. +4. ASAN is fully clean — no refcount-induced UAF/double-free/leak. A refcount bug + would crash (caught by ASAN) or corrupt broadly; it cannot selectively delete + one committed job's payload from a WAL. +5. No chaotic soak EVER reached rounds 4-6 before (every prior run crashed/stalled + at round 3-4: bug-851 cross-deny → bug-1282 deadlock → bug-1286 segv → 5.5/5.6/ + 5.7 election stalls → bug-1298 UAF). There is no prior green baseline for rounds + 4-6 to regress FROM — this is a first observation. + +Conclusion: bug-795/828/851 FAILOVER CATCH-UP GAP — a restarted node rejoins and is +elected primary without backfilling the full committed prefix (empty-rejoin catch-up +streams [watermark..committed], no snapshot backfill → permanent gap). EXTERNAL spec +ha-commit-durability (+ raft term/log-safety). Tracked as task 5.9; route there. diff --git a/.kiro/specs/raft-ha-core-safety/tasks.md b/.kiro/specs/raft-ha-core-safety/tasks.md index 753eeb05..a869925f 100644 --- a/.kiro/specs/raft-ha-core-safety/tasks.md +++ b/.kiro/specs/raft-ha-core-safety/tasks.md @@ -154,15 +154,23 @@ - _Boundary: Election Liveness_ - _Depends: 5.6_ -- [ ] 5.8 Endpoint teardown-vs-dispatch heap-use-after-free crash (NEW — the round-4 7.2 blocker after 5.7, UPSTREAM-owned) - - _Root cause (`research.md` §"Task 5.7 fix VERIFIED" + bug-1298): with 5.7's election fix in, the 6-round chaotic soak's round-4 stall RE-ROOTS away from elections entirely. ctrl-3 ABORTs on an ASAN heap-use-after-free: the vtask dispatcher (T3) reads `ep->busy` via `endpoint_is_task_blocked` (netio.c:1008, in `vtask_eligible`→`dataq_deq_first_match`) on an endpoint that the io_worker (T2) just `free()`d via `endpoint_killer` (cluster.c:813, from `handle_endpoint_error`→`zarray_del`). The endpoint is freed while still referenced by a pending vtask in the dataq — no drain of its queued vtasks before the free, no refcount across the dispatcher read. ctrl-3 dies → cluster drops to 1/3 → no quorum → "no new primary within window." NOT an election bug; elections + job safety hold for every round that completes (rounds 1-3: zero job loss, bounded terms)._ - - **Ownership = UPSTREAM raft-drive-async / core libxce async-io dispatch** (vtaskmgr/dataq/netio/cluster teardown) — same surface and same routing precedent as task 5.4/bug-1282 (this spec's Non-Goals exclude the async-io layer). Same family as bug-1282 (ABBA, ep->busy made lockless) and the bug-1197 dispatch_inflight precedent; the bug-1282 lockless busy-read likely unmasked this latent UAF by letting the dispatcher reach the read sooner._ - - Fix direction (upstream): drain/cancel an endpoint's queued vtasks from the dataq BEFORE `endpoint_killer` frees it, OR hold a refcount/epoch on `endpoint_t` across the dispatcher eligibility read so the free defers until no dispatcher references remain (mirror bug-1197 `dispatch_inflight`). Must keep the bug-1282 lockless busy read and not reintroduce the ABBA ep->lock/q->lock inversion. - - Observable completion: the 6-round chaotic soak runs all rounds with NO controller crash — the green 7.2 gate +- [x] 5.8 Endpoint teardown-vs-dispatch heap-use-after-free crash (NEW — the round-4 7.2 blocker after 5.7) + - _FIXED + VALIDATED (commit 6e05dc1, `research.md` §"Task 5.8 fix VALIDATED"). With 5.7's election fix in, the round-4 stall RE-ROOTED off elections entirely: ctrl-3 ABORTed on an ASAN heap-use-after-free — the vtask dispatcher read `ep->busy` (endpoint_is_task_blocked, netio.c) on a queued vtask while the io_worker freed that ep via `endpoint_killer` (cluster.c, from handle_endpoint_error). ctrl-3 died → cluster 1/3 → no quorum → "no new primary."_ + - _FIX (chosen by user over the targeted deferred-teardown patch): a principled endpoint REFERENCE COUNT, consolidating the prior ad-hoc lifetime patches (bug-1282 lockless busy, bug-1197 dispatch_inflight). endpoint_t.refcnt (_Atomic) + heap_alloc; endpoint_get starts refcnt=1 (owner); vtask_enq takes a per-vtask ref BEFORE publishing the task to the dispatcher, run_task/taskq_item_killer release it; every heap-ep teardown site drops its owner ref via endpoint_unref (last unref fini+frees). Embedded eps (client ctx->ep, heap_alloc=0) are never freed by unref. So the dispatcher reads ep->busy on a queued vtask without the io_worker freeing it underneath — UAF eliminated. Also fixed several latent ep-struct leaks (endpoint_fini-only sites)._ + - _VALIDATION: controller_unit_test 182 tests / 0 failures (test_ha_vep_teardown updated — its OFF-path test asserted the now-fixed struct leak). 6-round ASAN chaotic soak: ZERO AddressSanitizer reports, ctrl-3 survives round 4, ALL 6 rounds elect at bounded terms (5→6→7→8). The round-4 crash-stall is gone._ + - Observable completion: the 6-round chaotic soak runs all rounds with NO controller crash ✅ (met). 7.2 still NOT green — see below. - _Requirements: 6.1, 6.2, 6.3_ - - _Boundary: Async-IO dispatch (UPSTREAM)_ + - _Boundary: Async-IO dispatch (endpoint lifetime)_ - _Depends: 5.7_ +- [ ] 5.9 Committed job loss + prefix divergence under chaotic failover (NEW — the round-4..6 7.2 blocker AFTER 5.8; pre-existing, EXTERNAL spec) + - _Surfaced by the FIRST soak ever to reach rounds 4-6 (every prior soak crashed at round 4 — bug-1282/1286/1298). With the crash gone, the 6-round chaotic soak FAILs the verdict: committed_job_loss=4, committed_divergence=7. Rounds 1-3 are CLEAN (all jobs present, zero divergence); loss begins exactly at round 4 — the first round whose overlapping kills elect a JUST-RESTARTED node. A committed job (e.g. 0-0-1782829181-1) is permanently absent on the reachable primary; the gap persists after settling. This is the documented bug-795/828/851 FAILOVER CATCH-UP GAP / committed-prefix divergence: a restarted node rejoins and is elected primary without backfilling the full committed prefix (empty-rejoin catch-up streams [watermark..committed], no snapshot backfill → permanent gap). NOT a 5.8 refcount regression: ASAN is clean (refcount governs ep lifetime, not WAL content; cannot selectively drop one committed job), and rounds 1-3 are loss-free._ + - **Ownership = EXTERNAL spec ha-commit-durability (+ raft term/log-safety).** Out of raft-ha-core-safety's election-liveness scope; route there. See `.kiro/specs/ha-commit-durability/INVESTIGATION-failover-job-loss.md` and the [[project_failover_job_loss]] / bug-851 memories. + - Observable completion: the 6-round chaotic soak verdict is committed_job_loss=0 AND committed_divergence=0 — the green 7.2 gate + - _Requirements: 6.1, 6.2, 6.3_ + - _Boundary: Commit durability / catch-up backfill (EXTERNAL)_ + - _Depends: 5.8_ + - [ ] 6. Integration: single-seam invariant end-to-end - [ ] 6.1 Verify leader→standby replication carries (term, payload) faithfully end-to-end - Integration check that the live leader replication AND the cross-term catch-up streaming path transmit each entry's full `(term, payload)` to the receiver: no `term==0` on the wire from a live leader (defensive), and — the real concern — no non-empty entry ever arrives with `PLEN=0`; the walAck catch-up continuation feeds only the seam ingest path diff --git a/.wolf/buglog.json b/.wolf/buglog.json index d68ec8d9..ad596753 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -21522,7 +21522,7 @@ "error_message": "AddressSanitizer: heap-use-after-free READ size 4 at endpoint_is_task_blocked netio.c:1008 (vtask dispatcher T3) — endpoint freed by endpoint_killer cluster.c:813 (io_worker T2); ctrl-3 ABORTs mid-chaotic-soak round 3→4, dropping the cluster to 1/3 (no quorum) so round 4 'NO new primary within window'", "file": "src/lib/netio.c / src/lib/vtaskmgr.c / src/controller/cluster.c", "root_cause": "Teardown-vs-dispatch race: io_worker (T2) tears down a connection on error — handle_endpoint_error → zarray_del → endpoint_killer free()s the endpoint_t — while the vtask dispatcher (T3) is concurrently scanning the dataq (dataq_deq_first_match → vtask_eligible → endpoint_is_task_blocked) and reads ep->busy on that just-freed endpoint. The endpoint is freed while still referenced by a pending vtask in the dataq; nothing drains/guards the ep's queued vtasks before the free, and no refcount keeps it alive across the dispatcher read. Same netio surface as bug-1282 (ABBA, ep->busy made lockless) and the bug-1197 dispatch_inflight precedent; the bug-1282 lockless read removed the (incidental) lock contention so the dispatcher reaches the read faster, exposing the latent UAF.", - "fix": "NOT FIXED — proposed: drain/cancel an endpoint's queued vtasks from the dataq BEFORE endpoint_killer frees it, or hold a refcount/epoch on endpoint_t across the dispatcher eligibility read so the free is deferred until no dispatcher references remain (mirror bug-1197 dispatch_inflight). Must preserve the bug-1282 lockless busy read and not reintroduce the ABBA ep->lock/q->lock inversion.", + "fix": "FIXED (commit 6e05dc1): added endpoint_t reference count (refcnt _Atomic + heap_alloc flag). endpoint_get starts refcnt=1 (owner); vtask_enq takes a per-vtask ref before publishing the task to the dispatcher, run_task/taskq_item_killer release it; every heap-ep teardown site drops its owner ref via endpoint_unref (last unref fini+frees). The dispatcher can now read ep->busy on a queued vtask without the io_worker freeing it underneath. Embedded eps (heap_alloc=0) never freed by unref. Validated: 182 unit tests/0 fail; 6-round ASAN chaotic soak ZERO ASan reports, ctrl-3 survives round 4, all rounds elect.", "tags": [ "netio", "vtaskmgr", @@ -21533,7 +21533,8 @@ "raft-ha-core-safety", "soak", "quorum", - "bug-1282-family" + "bug-1282-family", + "FIXED" ], "related_bugs": [ "bug-1282", @@ -21541,7 +21542,7 @@ "bug-1286" ], "occurrences": 1, - "last_seen": "2026-06-30T13:36:00-07:00" + "last_seen": "2026-06-30T14:25:00-07:00" }, { "id": "bug-1299", @@ -21558,6 +21559,102 @@ "related_bugs": [], "occurrences": 1, "last_seen": "2026-06-30T13:39:09.194Z" + }, + { + "id": "bug-1300", + "timestamp": "2026-06-30T14:04:05.998Z", + "error_message": "Significant refactor of ", + "file": "src/lib/netio.c", + "root_cause": "4 lines replaced/restructured", + "fix": "Rewrote 11→11 lines (4 removed)", + "tags": [ + "auto-detected", + "refactor", + "c" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-06-30T14:04:05.998Z" + }, + { + "id": "bug-1301", + "timestamp": "2026-06-30T14:05:02.703Z", + "error_message": "Significant refactor of ", + "file": "src/controller/cluster.c", + "root_cause": "2 lines replaced/restructured", + "fix": "Rewrote 9→12 lines (2 removed) | Also: /* zarray_del will finalize ep */; endpoint_fini(ep); | Also: endpoint_fini(ep2);; FREEUP(ep2);", + "tags": [ + "auto-detected", + "refactor", + "c" + ], + "related_bugs": [], + "occurrences": 4, + "last_seen": "2026-06-30T14:05:54.573Z" + }, + { + "id": "bug-1302", + "timestamp": "2026-06-30T14:07:16.602Z", + "error_message": "Significant refactor of ", + "file": "src/controller/ha.c", + "root_cause": "2 lines replaced/restructured", + "fix": "Rewrote 8→7 lines (2 removed) | Also: endpoint_fini(ep);; free(ep); | Also: endpoint_fini(ep);; free(ep); /* endpoint_get() calloc'd ep; ", + "tags": [ + "auto-detected", + "refactor", + "c" + ], + "related_bugs": [], + "occurrences": 3, + "last_seen": "2026-06-30T14:09:43.010Z" + }, + { + "id": "bug-1303", + "timestamp": "2026-06-30T14:14:35.148Z", + "error_message": "Significant refactor of ", + "file": "src/controller/testsuite/test_ha_vep_teardown.c", + "root_cause": "4 lines replaced/restructured", + "fix": "Rewrote 8→9 lines (4 removed) | Also: /* ep was not torn down by a vtask here — free it ; * by the drop path). */", + "tags": [ + "auto-detected", + "refactor", + "c" + ], + "related_bugs": [], + "occurrences": 2, + "last_seen": "2026-06-30T14:16:12.183Z" + }, + { + "id": "bug-1304", + "timestamp": "2026-06-30T14:24:36.874Z", + "error_message": "Function not marked async", + "file": ".kiro/specs/raft-ha-core-safety/research.md", + "root_cause": "Function uses await but wasn't declared async", + "fix": "Added async modifier", + "tags": [ + "auto-detected", + "async-fix", + "md" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-06-30T14:24:36.874Z" + }, + { + "id": "bug-1305", + "timestamp": "2026-06-30T14:25:55.429Z", + "error_message": "Missing error handling in unknown", + "file": ".kiro/specs/raft-ha-core-safety/tasks.md", + "root_cause": "Code path had no error handling — exceptions would propagate uncaught", + "fix": "Added try/catch block", + "tags": [ + "auto-detected", + "error-handling", + "md" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-06-30T14:25:55.429Z" } ] } \ No newline at end of file