From 9e28defcab27abc28782f2225772c00dff74c131 Mon Sep 17 00:00:00 2001 From: Will Anderson Date: Wed, 12 Aug 2026 18:27:13 -0500 Subject: [PATCH] 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. --- lang/runtime/engram_store.c | 38 +++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/lang/runtime/engram_store.c b/lang/runtime/engram_store.c index 017e393..a95ab6e 100644 --- a/lang/runtime/engram_store.c +++ b/lang/runtime/engram_store.c @@ -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;