fix(raft-drive-async): endpoint-quiescence for bug-1197 vote-ep teardown (task 3.4a)
When the (still-default-OFF) peer-control dispatch flag is ON, raft_cb_become_follower runs as a vtask pool worker and frees every peer vote-ep while other pool workers may still hold those eps' data — a UAF (bug-1197). This makes that path quiescence-safe so the flag can later be flipped ON (deferred to the 4.x prod-flip gate; production stays OFF here). Mechanism (no libxce-wide endpoint refcount; reuses the existing per-ep FIFO + busy serialization): - Deferred teardown-as-vtask: the loser loop, when dispatch is ON and the pool is wired, does per vote-ep endpoint_mark_tearing_down -> iomgr_unmonitor -> enqueue a VTASK_IO/VTASK_LIVENESS teardown vtask bound to that ep. Per-ep busy FIFO makes it run strictly after any in-flight/queued same-ep peer-control vtask; the body NULLs iod->ep before endpoint_fini+free(ep) to disarm run_task's post-body endpoint_unblock_tasks (which would touch the freed ep->lock). OFF / pool-unwired path is byte-for-byte the legacy endpoint_fini. - RCU-like drain closes the dispatch/teardown TOCTOU window: new _Atomic int endpoint_t.dispatch_inflight (+ tearing_down:1 bit). The ON dispatch brackets its flag-check+enqueue with fetch_add/fetch_sub (seq_cst); endpoint_mark_tearing_down sets the flag under ep->lock, releases, then spins until dispatch_inflight drains to 0. So every dispatch that read the flag false finishes its enqueue before mark returns (FIFO-before the teardown vtask); any later dispatch reads the flag set and drops the packet. ep->lock is deliberately NOT held across the enqueue (it inverts with the taskq lock in vtask_eligible); the counter is the lock-free handshake. Drain is bounded: the held window is just a calloc + non-blocking enqueue on the single io_worker. Still open before the production flag flip (4.x): the borrowed-ep race in snapshot_lane_fn/configpush_lane_fn is distinct (slowlane thread vs io_worker RDHUP teardown, not the vtask-pool FIFO) and needs its own per-standby-ep discipline. Tests: test_ha_vep_teardown.c — 5 ASan tests (ordering, late-event drop, run_task unblock disarm, OFF-path unchanged, deterministic mark-drains-inflight-dispatch). xcelib_test 282/0; controller_unit_test EXIT=0; ASan-clean. Adversarial cpp-review: no CRITICAL/HIGH; handshake formally sound, flag-ON path race-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S6fDCtXkkEgupu7rUah1y2
This commit is contained in:
parent
bd02064ebc
commit
6d510d8cf4
10 changed files with 1037 additions and 10 deletions
|
|
@ -103,7 +103,8 @@ Tasks mirror the design's 4-phase migration. The slow lane is foundational (ever
|
|||
- _Boundary: PeerCtrlDispatch, SlowLane_
|
||||
- _Depends: 3.1_
|
||||
|
||||
- [ ] 3.4 Receiver-side integration tests
|
||||
- [ ] 3.4 Receiver-side integration tests <!-- 3.4a bug-1197 endpoint-quiescence fix DONE (committed); 3.4b integration tests + prod flag-flip PENDING — see Implementation Notes -->
|
||||
- **3.4a (DONE)**: bug-1197 endpoint-quiescence fix landed so flag-ON peer-control dispatch is UAF-free. Production flags STILL DEFAULT OFF (per the "full fix, defer prod flip" decision). The prod flip happens only after 4.1 partition soak + 4.2 regression sweep pass.
|
||||
- Assert that during a snapshot/config-push transfer the controller still processes other peers' heartbeats and votes; that a quorum of WAL-acks advances the commit position promptly even with the vtask pool near saturation; and that cross-endpoint acks are not serialized behind one another.
|
||||
- Observable: tests pass; commit advances after a quorum of acks under load; iomgr stays responsive during transfers.
|
||||
- _Requirements: 1.1, 1.3, 1.4, 4.3, 4.5_
|
||||
|
|
@ -150,3 +151,9 @@ Tasks mirror the design's 4-phase migration. The slow lane is foundational (ever
|
|||
- **WHY DORMANT (flag-ON gate, owned by 3.4)**: `snapshot_lane_fn` captures a BORROWED standby endpoint (unlike reconnect, which owns its ep). With the flag ON the send runs on a lane thread while the io_worker may RDHUP-`endpoint_fini` that ep → cross-thread UAF (bug-1197 family). Flag OFF keeps send+teardown serialized on the io_worker. **3.4 is the single gate** that flips BOTH `g_drive_async_snapshot_offload` AND `g_drive_async_peerctrl_dispatch` ON, after establishing the endpoint-lifetime discipline (refcount or lane-serialized teardown) and proving liveness under pool saturation. Caught + fixed pre-commit: the offload flag was initially authored DEFAULT-ON (would have shipped the UAF) → corrected to OFF in both ha.c and the ha.h accessor doc (bug-1210).
|
||||
|
||||
- **3.3 (landed DORMANT — checkbox `[~]` until 3.4 gate)**: requester-less config-push transport offload, flag `g_drive_async_configpush_offload` **DEFAULT 0/OFF** (toggle `ha_set_configpush_offload_for_test`). `ha_config_push_offloadable(c, bytes, blen, sha256, new_tag)` routes inline (flag OFF — verbatim `push_config_transport_only`, caller still owns+frees `bytes`) or to `slowlane_submit`+`configpush_lane_fn` (flag ON — DEEP-COPIES `bytes`+sha into a ctx the lane frees). Single cluster-wide single-flight via scalar `ha_state_t.configpush_inflight` CAS + lane key=1 (config push is a cluster-wide fan-out, not per-peer); flag cleared on all terminal paths (alloc-fail×2, -EEXIST, hard-error, lane completion). Send body factored into `ha_config_push_do_send` (production → `push_config_transport_only`; XCE_TESTING → `ha_configpush_probe` stub since cluster.c/transport isn't linked in test). THREE requester-less drive-thread call sites converted: `raft_cb_become_leader` (promotion, both bytes/NULL branches), the `HA_REPL_PUSH_CONFIG` heartbeat case, and the direct-CTRL_PRIMARY FSM promotion branch (~ha.c:4476). UNCHANGED: `push_config_to_standbys` admin-upgrade barrier (has a waiting requester) and `join.c`. Tests: `test_configpush_offload_runs_on_lane`, `test_configpush_inline_when_flag_off`, `test_configpush_offload_dedups` (test_ha_liveness.c). Same BORROWED-ENDPOINT/bug-1197 dormancy rationale as 3.2 (lane reads `c->ha.standby_endpoints`); **3.4 flips this ON together with snapshot + peerctrl flags**. cpp-review: no CRITICAL/HIGH/MEDIUM; controller suite EXIT=0.
|
||||
|
||||
- **3.4a (bug-1197 endpoint-quiescence fix — committed; flags STILL default OFF)**: makes the flag-ON peer-control dispatch path UAF-free so `raft_cb_become_follower` (running as a pool vtask) can free peer vote-eps while other pool workers process peer-control on the same eps. Mechanism (no libxce-wide endpoint refcount; reuses the existing per-ep FIFO + busy serialization):
|
||||
- **Deferred teardown-as-vtask**: the loser loop, when `ha_peerctrl_dispatch_enabled() && ha->pool`, does per vote-ep `endpoint_mark_tearing_down(ep)` → `iomgr_unmonitor(ep)` → enqueue a `VTASK_IO`/`VTASK_LIVENESS` teardown vtask bound to that ep (`vote_ep_teardown_vtask_entry`). Per-ep busy FIFO makes it run strictly AFTER any in-flight/queued same-ep peer-control vtask; it NULLs `iod->ep` before `endpoint_fini`+`free(ep)` to disarm `run_task`'s post-body `endpoint_unblock_tasks` (which would otherwise touch the freed `ep->lock`). OFF / pool-unwired path is byte-for-byte the legacy `endpoint_fini` (no free).
|
||||
- **RCU-like drain closes the TOCTOU window**: a new `_Atomic int endpoint_t.dispatch_inflight` (+ `tearing_down:1` bit). The ON dispatch brackets its flag-check+enqueue: `fetch_add(seq_cst)` → if `tearing_down` drop pkt + `fetch_sub` → else enqueue → `fetch_sub`. `endpoint_mark_tearing_down` sets the flag under `ep->lock`, releases, then spins `while(load(dispatch_inflight,seq_cst)>0) sched_yield()`. become_follower marks (drains) BEFORE enqueuing the teardown vtask, so any dispatch that read the flag false is FIFO-before teardown; any later dispatch drops. Could NOT hold `ep->lock` across the enqueue — it inverts with the taskq lock (dispatcher takes taskq→ep in `vtask_eligible`); the counter is the lock-free handshake. Drain is bounded (the held window is just calloc+non-blocking enqueue on the single io_worker).
|
||||
- Files: `src/include/netio.h` (`tearing_down:1`, `dispatch_inflight`, mark/is helpers), `src/lib/netio.c` (helpers + drain + `do_endpoint_init` zero-init), `src/controller/ha.c` (`vote_ep_teardown_*`, loser-loop async branch, dispatch bracket + late-drop, test seams), `src/controller/ha.h`. Tests: `test_ha_vep_teardown.c` (5 ASan tests: ordering, late-drop, disarm, OFF-path, deterministic drain-blocks). xcelib_test 282/0; controller EXIT=0; ASan-clean. Adversarial cpp-review: no CRITICAL/HIGH; handshake formally sound (SC total order on the counter + monotone `tearing_down` under `ep->lock`); flag-ON path proven race-free.
|
||||
- **STILL OPEN for the prod flip (4.x)**: the borrowed-ep hazard in `snapshot_lane_fn`/`configpush_lane_fn` is a DISTINCT race (slowlane thread vs io_worker RDHUP teardown, not the vtask-pool FIFO) — needs its own per-standby-ep refcount/lane-serialized teardown before `g_drive_async_snapshot_offload`/`configpush_offload` go ON in production. 3.4b integration tests may enable the flags in-test (no mid-transfer ep teardown, so the lane race isn't exercised).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue