Skip to content

fix(mpool): cache shrink detached one region past the end (SIGSEGV) - #83

Merged
gburd merged 1 commit into
masterfrom
fix/memp-shrink-region-index
Jul 29, 2026
Merged

fix(mpool): cache shrink detached one region past the end (SIGSEGV)#83
gburd merged 1 commit into
masterfrom
fix/memp-shrink-region-index

Conversation

@gburd

@gburd gburd commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Real crash bug found by the MVCC/cache-resize coverage work (PR #81).

__memp_remove_region detached dbmp->reginfo[mp->nreg] — but live cache regions occupy reginfo[0 .. nreg-1], so reginfo[nreg] is one slot past the last region. __env_region_detach then munmap()'d that stale/garbage REGINFO, reliably SIGSEGV'ing (in __os_detach) whenever the cache was shrunk to fewer regions (DB_ENV->resize_cache / memp_resize downward).

It was a mirror-image copy-paste of the add path: __memp_add_region correctly uses reginfo[mp->nreg] because it's initializing the next, not-yet-counted slot before mp->nreg++. Removal is the opposite — it must index the last live region (reginfo[nreg-1]), then decrement.

Fix: detach reginfo[mp->nreg - 1].

Reproduction (now fixed)

set e [berkdb_env -create -txn -cache_max {0 16777216} -cachesize {0 1048576 2} -home $dir]
# ... populate ...
$e resize_cache {0 4194304}   ;# grow  — OK
$e resize_cache {0 2097152}   ;# shrink — was SIGSEGV, now clean

Verified

  • Grow-then-shrink completes cleanly (was SIGSEGV in __os_detach)
  • Data survives repeated grow/shrink cycles intact
  • memp001/003 + mvcc001 pass

This is the 3rd real bug surfaced by the coverage program (after #67 heap-regression and #77 upstream __db_set_lastpgno off-by-one).

__memp_remove_region detached dbmp->reginfo[mp->nreg] -- but live cache regions
occupy reginfo[0 .. nreg-1], so reginfo[nreg] is one slot PAST the last region.
__env_region_detach then munmap()'d that stale/garbage REGINFO, reliably
crashing (SIGSEGV in __os_detach) whenever the cache was shrunk to fewer
regions (DB_ENV->resize_cache / memp_resize downward).

The bug was a mirror-image copy of the add path: __memp_add_region correctly
uses reginfo[mp->nreg] because it is initializing the NEXT, not-yet-counted slot
before doing mp->nreg++.  Removal is the opposite -- it must index the last
LIVE region, reginfo[nreg-1], and then decrement nreg.

Fix: detach reginfo[mp->nreg - 1].

Found by the MVCC/cache-resize coverage work (PR #81), which grows-only to dodge
this crash.

Verified: grow-then-shrink (the documented repro) now completes cleanly instead
of SIGSEGV; data survives repeated grow/shrink cycles; memp001/003 + mvcc001 pass.
@gburd
gburd merged commit 7f63730 into master Jul 29, 2026
47 of 49 checks passed
@gburd
gburd deleted the fix/memp-shrink-region-index branch July 29, 2026 12:53
@github-actions

Copy link
Copy Markdown

Coccinelle convention checks

No new violations. ✅

Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in.
rule_mutex_unbalanced|MUTEX_UNBALANCED|src/crypto/mersenne/mt19937db.c|return (ret);
rule_mutex_unbalanced|MUTEX_UNBALANCED|src/mp/mp_register.c|return (ret);

@github-actions

Copy link
Copy Markdown

ABI diff vs v5.3.31 (libabigail — authoritative)

Removed exported symbols (nm -D, _NNNN version suffix normalized)

None.


Advisory: libabigail/nm is the authoritative binary-ABI check; Coccinelle is complementary source-level early warning. See dist/cocci/README.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant