fix(controller): CtrlHelloRequest.key Data field crashed HA leader election (bug-1835)
CtrlHelloRequest.key was declared `Data` in controller.capnp. This project's capnp-c backend silently no-ops encoding of Data-typed fields without zero-initializing the caller's local struct, leaving an uninitialized capn_ptr that write_CtrlHelloRequest unconditionally wrote to the wire -- SIGSEGV in capn_setp/copy_ptr. Hit live: ctrl-1 won its Raft election and crashed calling ha_send_ctrlhello_to_all_standbys(). Same bug class as bug-1504/bug-1505, both already fixed by switching the field from Data to List(UInt8); bug-1505's own writeup had flagged CtrlHelloRequest.key as a known-but-unfixed instance. Applied the identical fix here: Data -> List(UInt8) + keyLen companion in the schema, uint8_t*/key_len in typemap.h. The field is unused in production (always NULL), so no consumer code changed. Added a round-trip regression test covering both the NULL-key production shape and real key content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
1e43345b2e
commit
cd9ecb4159
7 changed files with 197 additions and 21 deletions
|
|
@ -1,13 +1,13 @@
|
|||
{
|
||||
"version": 1,
|
||||
"meta": {
|
||||
"lastScanned": "2026-09-20T13:14:29.254Z",
|
||||
"fileCount": 665,
|
||||
"lastScanned": "2026-09-21T03:39:53.909Z",
|
||||
"fileCount": 666,
|
||||
"hits": 0,
|
||||
"misses": 0,
|
||||
"renderedHash": "858015d3083ccb0ba678b885dd6c20d8b4487c0043a691ea447f0c97ee9f9308",
|
||||
"storeUpdatedAt": "2026-09-20T13:14:29.263Z",
|
||||
"rootHash": "e1e5eae9e6a128bf"
|
||||
"renderedHash": "45c99420f9851c89e6b8a493a3c4c5f0a05400f4342a6cd486d0b86ccf3680a8",
|
||||
"storeUpdatedAt": "2026-09-21T03:39:53.919Z",
|
||||
"rootHash": "5dc33e10712cf857"
|
||||
},
|
||||
"files": {
|
||||
".agent-shell/transcripts/2026-08-11-11-39-59.md": {
|
||||
|
|
@ -10430,20 +10430,20 @@
|
|||
},
|
||||
"src/schema/controller.capnp": {
|
||||
"description": "Declares OPCode",
|
||||
"tokens": 16126,
|
||||
"hash": "90b5b72153c11857",
|
||||
"size": 61231,
|
||||
"mtimeMs": 1789566368210.08,
|
||||
"updatedAt": "2026-09-16T13:46:08.258Z",
|
||||
"tokens": 16389,
|
||||
"hash": "9689b05d99cd0023",
|
||||
"size": 62218,
|
||||
"mtimeMs": 1789961885889.9292,
|
||||
"updatedAt": "2026-09-21T03:38:05.932Z",
|
||||
"source": "hook"
|
||||
},
|
||||
"src/include/typemap.h": {
|
||||
"description": "if !defined(_TYPEMAP_H_)",
|
||||
"tokens": 8664,
|
||||
"hash": "e67c46e54df6e1f8",
|
||||
"size": 32933,
|
||||
"mtimeMs": 1789566375907.337,
|
||||
"updatedAt": "2026-09-16T13:46:15.946Z",
|
||||
"tokens": 8720,
|
||||
"hash": "f038a62e2ac8f43a",
|
||||
"size": 33146,
|
||||
"mtimeMs": 1789961896088.1375,
|
||||
"updatedAt": "2026-09-21T03:38:16.133Z",
|
||||
"source": "hook"
|
||||
},
|
||||
"src/controller/ha.h": {
|
||||
|
|
@ -12207,6 +12207,15 @@
|
|||
"mtimeMs": 1789910069218.0444,
|
||||
"updatedAt": "2026-09-20T13:14:29.254Z",
|
||||
"source": "hook"
|
||||
},
|
||||
"src/controller/testsuite/test_e2e_capnp.c": {
|
||||
"description": "include <stdint.h>",
|
||||
"tokens": 20187,
|
||||
"hash": "63c314d2bcebbc60",
|
||||
"size": 75708,
|
||||
"mtimeMs": 1789961993857.0706,
|
||||
"updatedAt": "2026-09-21T03:39:53.909Z",
|
||||
"source": "hook"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
# anatomy.md
|
||||
|
||||
> Auto-maintained by OpenWolf. Last scanned: 2026-09-20T13:14:29.254Z
|
||||
> Files: 665 tracked | Anatomy hits: 0 | Misses: 0
|
||||
> Auto-maintained by OpenWolf. Last scanned: 2026-09-21T03:39:53.909Z
|
||||
> Files: 666 tracked | Anatomy hits: 0 | Misses: 0
|
||||
|
||||
## ./
|
||||
|
||||
|
|
@ -893,6 +893,7 @@
|
|||
- `clean_failover_preservation.sh` — clean_failover_preservation.sh — ha-term-safety task 5.2 live integration test. (~4972 tok)
|
||||
- `controller_test.c` — include <argp.h> (~2466 tok)
|
||||
- `test_ctrlstatus.c` — include <stdatomic.h> (~12594 tok)
|
||||
- `test_e2e_capnp.c` — include <stdint.h> (~20187 tok)
|
||||
- `test_ha_heartbeat_committed.c` — include <stdatomic.h> (~5775 tok)
|
||||
- `test_ha_liveness.c` — Declares directly (~25885 tok)
|
||||
- `test_ha_partition.c` (~5500 tok)
|
||||
|
|
@ -916,7 +917,7 @@
|
|||
|
||||
- `request.h` — if !defined(_REQUEST_H_) (~3288 tok)
|
||||
- `resreq.h` — if !defined(_RESREQ_H_) (~3310 tok)
|
||||
- `typemap.h` — if !defined(_TYPEMAP_H_) (~8664 tok)
|
||||
- `typemap.h` — if !defined(_TYPEMAP_H_) (~8720 tok)
|
||||
- `utils.h` — if !defined(_UTILS_H_) (~3454 tok)
|
||||
- `xcelimit.h` — if !defined(_XCELIMIT_H_) (~428 tok)
|
||||
|
||||
|
|
@ -1040,7 +1041,7 @@
|
|||
|
||||
## src/schema/
|
||||
|
||||
- `controller.capnp` — Declares OPCode (~16126 tok)
|
||||
- `controller.capnp` — Declares OPCode (~16389 tok)
|
||||
|
||||
## src/tools/
|
||||
|
||||
|
|
|
|||
|
|
@ -24742,6 +24742,40 @@
|
|||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-09-20T10:15:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1835",
|
||||
"timestamp": "2026-09-21T03:47:10Z",
|
||||
"error_message": "Live 3-node HA cluster crash: xce_controller SIGSEGV in __memcpy_avx_unaligned_erms, called from capnp's copy_ptr/capn_setp, called from write_CtrlHelloRequest (lib/controller.capnp.c:22821) via encode_CtrlHelloRequest_ptr (lib/controller.capnp.c:22935), via xce_encode/xce_request, via ha_send_ctrlhello_to_all_standbys (controller/cluster.c:2432), via raft_cb_become_leader (controller/ha.c:2113), via raft_on_vote_reply -> vote_reply_dispatch -> peer_ctrl_msg_handler_inline -> peer_ctrl_vtask_entry, on the vcpu thread handling the winning vote reply. Core confirmed built from commit 1e43345b (xce v0.0.1-1e43345b, 2026-09-21T03:10:39Z); ctrl-1 had just won its Raft election (term 2, votes_granted=2) and crashed while broadcasting CtrlHello to its 2 standbys.",
|
||||
"file": "src/schema/controller.capnp",
|
||||
"root_cause": "CtrlHelloRequest.key was declared `Data` in controller.capnp -- the exact same latent bug class as bug-1504 (SnapshotData.internalState) and bug-1505 (WalCatchupEntry.payload), and explicitly named as a known-but-unfixed instance of it in bug-1505's own writeup ('Known/suspected other Data fields with the same latent gap: ... CtrlHelloRequest.key'). This project's capnp-c backend silently no-ops the encoding of any Data-typed field inside the generated encode_XXX() composite helper (see encode_CtrlHelloRequest's ' /* key key */' stub, lib/controller.capnp.c) without ever zero-initializing the caller's local `struct CtrlHelloRequest d;` first (encode_CtrlHelloRequest_ptr, lib/controller.capnp.c:22926-22938) -- so d.key.p is left as genuinely uninitialized stack garbage. write_CtrlHelloRequest (lib/controller.capnp.c:22821) then unconditionally calls capn_setp(p.p, 1, s->key.p) with that garbage capn_ptr, and capn_setp's copy_ptr dereferences the garbage segment/data pointer, SIGSEGV'ing in memcpy. ha_send_ctrlhello_to_all_standbys (controller/cluster.c:2419-2424) and ha.c:9495 both always set ctrl_hello_req_t.key = NULL (the field has never had a real producer anywhere in the codebase -- confirmed via project-wide grep, no call site ever sets it to non-NULL nor reads it back), so this is genuine undefined behavior whose manifestation depends entirely on whatever garbage bytes happen to occupy that vtask thread's stack slot at each call -- exactly the same 'lucky vs unlucky stack layout' nondeterminism documented for bug-1504/1505, which is why this had not crashed before despite the gap being known since 2026-08-07. Also confirmed the C struct itself compounded the risk: ctrl_hello_req_s.key (typemap.h) was typed `xce_data_t*` -- the local-Data-struct-shadowing wrapper type from bug-1461's own root cause -- rather than a real byte-blob type, even though it was never actually used as one.",
|
||||
"fix": "FIXED (2026-09-21), same pattern as bug-1504/bug-1505: changed CtrlHelloRequest.key from `Data` to `List(UInt8)` in src/schema/controller.capnp (which this capnp-c backend DOES correctly auto-generate real, non-no-op encode/decode/free for -- proven precedent: FetchFileReply.content/bytesRead, SnapshotData.internalState, WalCatchupEntry.payload), with a new keyLen (@4, UInt32) companion field as its maplistcount. Updated typemap.h's ctrl_hello_req_s.key from `xce_data_t*` to `uint8_t*` and added a new `key_len` (uint32_t) field. No consumer-code changes were needed: project-wide grep confirmed .key is never set to non-NULL nor read anywhere in production (ha_send_ctrlhello_to_all_standbys and ha.c's hello builder both always pass NULL), so the fix is purely structural -- it eliminates the uninitialized-capn_ptr class of UB for this field rather than papering over one occurrence. Added regression test test_ctrl_hello_request_key_round_trip (controller/testsuite/test_e2e_capnp.c), covering both the actual production shape (key==NULL/key_len==0, the exact shape that used to SIGSEGV -- now proven to encode/decode cleanly via the real generic codec) and a real non-NULL key payload (proves content genuinely round-trips now, which the old no-op'd Data codec never did). A deterministic RED test reproducing the SIGSEGV itself was not attempted -- by its own nature as reads of uninitialized stack memory, this bug class is not ASAN-catchable and its manifestation is stack-layout-dependent (bug-1504's own writeup makes the identical observation); the fix instead structurally eliminates the uninitialized-read entirely rather than chasing a flaky repro. Verified: full controller_unit_test suite (5/5 meson test binaries, 0 failures, including the new round-trip test and the pre-existing test_ctrl_hello_req_t_layout/test_ha_ctrlhello.c suite) via a clean ASAN rebuild (`xce-build.sh controller asan`) that also regenerated controller.capnp.c/.h from the updated schema with no codegen errors.",
|
||||
"tags": [
|
||||
"controller",
|
||||
"ha",
|
||||
"capnp-schema",
|
||||
"codegen-limitation",
|
||||
"sigsegv",
|
||||
"uninitialized-memory",
|
||||
"controller.capnp",
|
||||
"ctrl-hello",
|
||||
"raft",
|
||||
"leader-election",
|
||||
"systemic",
|
||||
"user-reported",
|
||||
"live-crash",
|
||||
"related-to-bug-1504",
|
||||
"related-to-bug-1505",
|
||||
"related-to-bug-1461",
|
||||
"fixed"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1504",
|
||||
"bug-1505",
|
||||
"bug-1461"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-09-21T03:47:10Z"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5524,3 +5524,12 @@
|
|||
| 04:53 | Edited src/lib/testsuite/test_limit.c | added 2 condition(s) | ~1989 |
|
||||
| 04:53 | Edited src/lib/testsuite/test_limit.c | expanded (+8 lines) | ~135 |
|
||||
| 06:14 | Created reports/sep13-sep19-2026.md | — | ~3659 |
|
||||
|
||||
## Session: 2026-09-21 20:31
|
||||
|
||||
| Time | Action | File(s) | Outcome | ~Tokens |
|
||||
|------|--------|---------|---------|--------|
|
||||
| 20:38 | Edited src/schema/controller.capnp | modified mapname() | ~329 |
|
||||
| 20:38 | Edited src/include/typemap.h | 7→10 lines | ~151 |
|
||||
| 20:39 | Edited src/controller/testsuite/test_e2e_capnp.c | modified test_ctrl_hello_request_key_round_trip() | ~1131 |
|
||||
| 20:39 | Edited src/controller/testsuite/test_e2e_capnp.c | 2→3 lines | ~46 |
|
||||
|
|
|
|||
|
|
@ -273,6 +273,111 @@ static void test_wal_catchup_append_term_fields_round_trip(void)
|
|||
printf("test_wal_catchup_append_term_fields_round_trip: PASS\n");
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* CtrlHelloRequest.key round-trip through the generic codec, now that */
|
||||
/* it's List(UInt8) instead of Data -- this is the exact field/path that */
|
||||
/* SIGSEGV'd a live 3-node HA cluster (write_CtrlHelloRequest writing an */
|
||||
/* uninitialized capn_ptr for a no-op'd Data field, hit the moment a */
|
||||
/* newly-elected primary called ha_send_ctrlhello_to_all_standbys()). */
|
||||
/* bug-1505's own writeup had already flagged CtrlHelloRequest.key as a */
|
||||
/* known-but-unfixed instance of the same bug class as bug-1504/bug-1505 */
|
||||
/* above; see controller.capnp's CtrlHelloRequest.key comment for the */
|
||||
/* full root cause. Covers both the actual production shape (key==NULL, */
|
||||
/* the only value any current caller ever sets -- must not crash) and a */
|
||||
/* real non-NULL key, to prove the codec genuinely carries content now */
|
||||
/* rather than silently dropping it. */
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
||||
static void test_ctrl_hello_request_key_round_trip(void)
|
||||
{
|
||||
/* Case 1: key == NULL (matches ha_send_ctrlhello_to_all_standbys and
|
||||
* ha.c's hello builder -- the ONLY shape any production caller uses
|
||||
* today, and exactly what used to SIGSEGV via uninitialized stack). */
|
||||
ctrl_hello_req_t src;
|
||||
memset(&src, 0, sizeof(src));
|
||||
src.name = "ctrl-1";
|
||||
src.key = NULL;
|
||||
src.key_len = 0;
|
||||
src.applied_lsn = 100ULL;
|
||||
src.config_tag = 55ULL;
|
||||
|
||||
struct capn c;
|
||||
capn_init_malloc(&c);
|
||||
struct capn_segment *cs = capn_root(&c).seg;
|
||||
|
||||
CtrlHelloRequest_ptr p;
|
||||
encode_CtrlHelloRequest_ptr(cs, &p, &src);
|
||||
capn_setp(capn_root(&c), 0, p.p);
|
||||
|
||||
int64_t sz = capn_size(&c);
|
||||
TEST_ASSERT_TRUE(sz > 0);
|
||||
uint8_t *buf = (uint8_t *)malloc((size_t)sz);
|
||||
TEST_ASSERT_NOT_NULL(buf);
|
||||
capn_write_mem(&c, buf, (size_t)sz, 0);
|
||||
capn_free(&c);
|
||||
|
||||
struct capn d;
|
||||
TEST_ASSERT_EQUAL_INT(0, capn_init_mem(&d, buf, (size_t)sz, 0));
|
||||
CtrlHelloRequest_ptr dp;
|
||||
dp.p = capn_getp(capn_root(&d), 0, 1);
|
||||
struct CtrlHelloRequest wire;
|
||||
read_CtrlHelloRequest(&wire, dp);
|
||||
ctrl_hello_req_t dst;
|
||||
memset(&dst, 0, sizeof(dst));
|
||||
decode_CtrlHelloRequest(&dst, &wire);
|
||||
|
||||
TEST_ASSERT_EQUAL_UINT64(100ULL, dst.applied_lsn);
|
||||
TEST_ASSERT_EQUAL_UINT64(55ULL, dst.config_tag);
|
||||
TEST_ASSERT_EQUAL_UINT32(0, dst.key_len);
|
||||
TEST_ASSERT_NULL(dst.key);
|
||||
|
||||
free_CtrlHelloRequest(&dst);
|
||||
capn_free(&d);
|
||||
free(buf);
|
||||
|
||||
/* Case 2: non-NULL key -- proves the wire genuinely carries content
|
||||
* now, which the old no-op'd Data codec never did. */
|
||||
static const uint8_t key_bytes[] = {
|
||||
0xDE, 0xAD, 0xBE, 0xEF, 0x01, 0x02, 0x03, 0x04
|
||||
};
|
||||
memset(&src, 0, sizeof(src));
|
||||
src.name = "ctrl-2";
|
||||
src.key = (uint8_t *)key_bytes;
|
||||
src.key_len = (uint32_t)sizeof(key_bytes);
|
||||
src.applied_lsn = 200ULL;
|
||||
src.config_tag = 99ULL;
|
||||
|
||||
capn_init_malloc(&c);
|
||||
cs = capn_root(&c).seg;
|
||||
encode_CtrlHelloRequest_ptr(cs, &p, &src);
|
||||
capn_setp(capn_root(&c), 0, p.p);
|
||||
|
||||
sz = capn_size(&c);
|
||||
TEST_ASSERT_TRUE(sz > 0);
|
||||
buf = (uint8_t *)malloc((size_t)sz);
|
||||
TEST_ASSERT_NOT_NULL(buf);
|
||||
capn_write_mem(&c, buf, (size_t)sz, 0);
|
||||
capn_free(&c);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, capn_init_mem(&d, buf, (size_t)sz, 0));
|
||||
dp.p = capn_getp(capn_root(&d), 0, 1);
|
||||
read_CtrlHelloRequest(&wire, dp);
|
||||
memset(&dst, 0, sizeof(dst));
|
||||
decode_CtrlHelloRequest(&dst, &wire);
|
||||
|
||||
TEST_ASSERT_EQUAL_UINT64(200ULL, dst.applied_lsn);
|
||||
TEST_ASSERT_EQUAL_UINT64(99ULL, dst.config_tag);
|
||||
TEST_ASSERT_EQUAL_UINT32((uint32_t)sizeof(key_bytes), dst.key_len);
|
||||
TEST_ASSERT_NOT_NULL(dst.key);
|
||||
TEST_ASSERT_EQUAL_MEMORY(key_bytes, dst.key, sizeof(key_bytes));
|
||||
|
||||
free_CtrlHelloRequest(&dst);
|
||||
capn_free(&d);
|
||||
free(buf);
|
||||
|
||||
printf("test_ctrl_hello_request_key_round_trip: PASS\n");
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* Test 6: Data union's new jobArray variant (job-array-single-wal-record */
|
||||
/* task 1.2, Req 6.1/6.2) round-trips the compact (base jobid, array size, */
|
||||
|
|
@ -1736,6 +1841,7 @@ void test_e2e_capnp(void)
|
|||
RUN_TEST(test_repl_envelope_capnp_fields);
|
||||
RUN_TEST(test_request_vote_last_log_tuple_round_trip);
|
||||
RUN_TEST(test_wal_catchup_append_term_fields_round_trip);
|
||||
RUN_TEST(test_ctrl_hello_request_key_round_trip);
|
||||
RUN_TEST(test_data_job_array_variant_round_trip);
|
||||
RUN_TEST(test_replay_header_snapshot_entry_round_trip);
|
||||
RUN_TEST(test_session_summary_round_trip);
|
||||
|
|
|
|||
|
|
@ -623,9 +623,12 @@ struct heartbeat_msg_s {
|
|||
/* CtrlHello state sync (Req 6.5, 6.6, 6.7) */
|
||||
struct ctrl_hello_req_s {
|
||||
char *name; /* standby's logical name */
|
||||
xce_data_t *key; /* SHA-256(cluster_name + shared_secret) auth key */
|
||||
uint8_t *key; /* SHA-256(cluster_name + shared_secret) auth key,
|
||||
* malloc'd, or NULL; wire type is List(UInt8),
|
||||
* not Data (see controller.capnp's own comment) */
|
||||
uint64_t applied_lsn; /* standby's last applied WAL LSN */
|
||||
uint64_t config_tag; /* standby's current config tag */
|
||||
uint32_t key_len; /* length of key */
|
||||
};
|
||||
|
||||
struct ctrl_hello_reply_s {
|
||||
|
|
|
|||
|
|
@ -1142,9 +1142,23 @@ struct WalCatchupEntry $C.mapname("wal_catchup_entry_t") {
|
|||
|
||||
struct CtrlHelloRequest $C.mapname("ctrl_hello_req_t") {
|
||||
name @0 :Text $C.mapname("name");
|
||||
key @1 :Data $C.mapname("key");
|
||||
# key is List(UInt8), NOT Data (fixes a live production SIGSEGV, same bug
|
||||
# class as bug-1504/bug-1505 -- see SnapshotData.internalState/
|
||||
# WalCatchupEntry.payload above for the full root cause: this codebase's
|
||||
# capnpc-c backend silently no-ops every Data-typed field's encoding in
|
||||
# the generated encode_XXX() helper without zero-initializing the
|
||||
# caller's local struct first, leaving a genuinely uninitialized capn_ptr
|
||||
# written onto the wire by write_XXX(). bug-1505's own writeup already
|
||||
# flagged "CtrlHelloRequest.key" as a known-but-unfixed instance of this
|
||||
# gap; it went on to actually crash a live 3-node HA cluster (SIGSEGV in
|
||||
# write_CtrlHelloRequest via encode_CtrlHelloRequest_ptr) the moment a
|
||||
# newly-elected primary called ha_send_ctrlhello_to_all_standbys() --
|
||||
# confirming the risk was real, not theoretical. keyLen (@4, below) is
|
||||
# its maplistcount companion.
|
||||
key @1 :List(UInt8) $C.mapname("key") $C.maplistcount("key_len");
|
||||
appliedLsn @2 :UInt64 $C.mapname("applied_lsn");
|
||||
configTag @3 :UInt64 $C.mapname("config_tag");
|
||||
keyLen @4 :UInt32 $C.mapname("key_len");
|
||||
}
|
||||
|
||||
struct CtrlHelloReply $C.mapname("ctrl_hello_reply_t") {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue