fix: bounds-check btree_insert + read_body to survive bloated store / WAL replay (live crash-loop root cause)

int_max_keys() computed an internal B+tree node's key capacity as
IDX_BODY/8 - 1 (2041 at a 16 KB page), dividing by 8 and ignoring that
each key also carries an 8-byte child pointer. The true capacity is
(IDX_BODY-8)/16 = 1020. An internal node was therefore allowed to grow to
~2x what a page holds; once it crossed 1020 keys, btree_insert's write-back
overran its STORE_PAGE_SIZE stack page buffer and smashed the stack canary
(__stack_chk_fail / SIGABRT). A clean/small store never grows an internal
node that large, so it never tripped; the ~8x-bloated live store (a day of
tombstone churn) plus a 44 MB un-checkpointed WAL replayed on open pushed a
node over the boundary during redo -> deterministic crash loop
(btree_insert <- apply_edge_put <- engram_open <- engram_store_boot).

Fixes:
- int_max_keys: use (IDX_BODY-8)/16 so internal nodes split at the real
  page capacity.
- btree_insert: reject any page whose on-disk nkeys exceeds physical
  capacity (fail loud, never smash the stack) -- overflow is now impossible
  regardless of on-disk content.
- read_body: bound the slot (off,len) and record length to the page before
  dereferencing; a stale/torn index entry could otherwise make store_get_node
  read off the stack (observed EXC_BAD_ACCESS on the bloated store). Fail safe.

Verified on a COPY of the live store: unfixed binary SIGABRTs in btree_insert
on open; fixed binary boots clean, recovers the store, checkpoints the WAL,
and M5 compaction shrinks 458 MB -> 57.7 MB with node/edge counts preserved.
This commit is contained in:
2026-08-12 18:27:13 -05:00
parent 1507614dbf
commit 9e28defcab
+36 -2
View File
@@ -574,8 +574,16 @@ static int leaf_max_entries(EngramPagedStore* s, uint32_t payload){
return nat;
}
static int int_max_keys(EngramPagedStore* s){
/* keys*8 + (keys+1)*8 <= IDX_BODY → keys <= IDX_BODY/8 - 1 */
int nat = (int)(IDX_BODY / 8) - 1;
/* An internal node stores `keys` u64 keys FOLLOWED BY (keys+1) u64 child
* pointers, so both arrays must fit the page body:
* keys*8 + (keys+1)*8 = 16*keys + 8 <= IDX_BODY → keys <= (IDX_BODY-8)/16.
* The prior form `IDX_BODY/8 - 1` divided by 8 instead of 16 — it counted
* only the key array and ignored the child array's 8 bytes/key — so it
* returned ~2x the real capacity (2041 vs 1020 at a 16 KB page). An internal
* node was then allowed to grow past what a page holds, and btree_insert's
* write-back overran its STORE_PAGE_SIZE stack page buffer, smashing the
* stack canary (__stack_chk_fail). That was the live crash-loop root cause. */
int nat = (int)((IDX_BODY - 8) / 16);
if (s->int_max > 0 && s->int_max < nat) return s->int_max;
return nat;
}
@@ -591,6 +599,22 @@ static int btree_insert(EngramPagedStore* s, int tree, uint64_t page_id,
int nkeys = get_u16(buf + 10);
int is_leaf = buf[IDX_LEAF_OFF];
/* Defensive bound: never trust an on-disk entry count enough to overflow the
* fixed STORE_PAGE_SIZE stack buffer below. A leaf holds at most IDX_BODY/esz
* entries; an internal node at most (IDX_BODY-8)/16 keys (keys + child ptrs).
* A page claiming more than its physical capacity is torn/corrupt (or was
* written by a pre-fix build) — fail LOUD and abort this insert rather than
* smash the stack or silently truncate. With this guard the memmove/memcpy/
* put_u64 write-backs are provably in-bounds regardless of on-disk content. */
int _cap = is_leaf ? (int)(IDX_BODY / esz) : (int)((IDX_BODY - 8) / 16);
if (nkeys < 0 || nkeys > _cap){
fprintf(stderr, "engram_store: corrupt %s index page %llu: nkeys=%d "
"exceeds page capacity %d — refusing insert (fail-safe)\n",
is_leaf ? "leaf" : "internal",
(unsigned long long)page_id, nkeys, _cap);
return -1;
}
if (is_leaf){
/* find insert position (after equal keys → stable duplicates) */
int pos = 0;
@@ -1013,8 +1037,17 @@ static int read_body(EngramPagedStore* s, uint64_t page, uint16_t slot,
uint16_t off,len,fl; slp_slot(buf, slot, &off, &len, &fl);
*live_out = (fl == SLOT_LIVE);
if (len < REC_HDR) return -1;
/* Defensive: the slot's (off,len) come from on-disk bytes. A stale primary
* index entry (churn/crash can leave one pointing at a page later repurposed)
* or a torn slot dir can yield an off/len that runs past this 16 KB stack page
* buffer — buf[off+..] would then read off the stack (observed EXC_BAD_ACCESS
* via store_get_node on the bloated store). Bound the record to the page and
* fail safe rather than over-read. */
if ((size_t)off + REC_HDR > STORE_PAGE_SIZE || (size_t)off + len > STORE_PAGE_SIZE)
return -1;
uint8_t rec_flags = buf[off + 3];
if (rec_flags & REC_OVERFLOW){
if ((size_t)off + REC_HDR + 16 > STORE_PAGE_SIZE) return -1; /* head+total u64s */
uint64_t head = get_u64(buf + off + REC_HDR);
uint64_t total = get_u64(buf + off + REC_HDR + 8);
uint8_t* body = ovf_read_chain(s, head, (size_t)total);
@@ -1022,6 +1055,7 @@ static int read_body(EngramPagedStore* s, uint64_t page, uint16_t slot,
*body_out = body; *blen_out = (size_t)total;
} else {
uint16_t reclen = get_u16(buf + off);
if (reclen < REC_HDR || (size_t)off + reclen > STORE_PAGE_SIZE) return -1;
size_t blen = reclen - REC_HDR;
uint8_t* body = (uint8_t*)malloc(blen ? blen : 1);
if (!body) return -1;