fix(mpool): cache shrink detached one region past the end (SIGSEGV) - #83
Merged
Conversation
__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.
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in. |
ABI diff vs
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Real crash bug found by the MVCC/cache-resize coverage work (PR #81).
__memp_remove_regiondetacheddbmp->reginfo[mp->nreg]— but live cache regions occupyreginfo[0 .. nreg-1], soreginfo[nreg]is one slot past the last region.__env_region_detachthenmunmap()'d that stale/garbage REGINFO, reliably SIGSEGV'ing (in__os_detach) whenever the cache was shrunk to fewer regions (DB_ENV->resize_cache/memp_resizedownward).It was a mirror-image copy-paste of the add path:
__memp_add_regioncorrectly usesreginfo[mp->nreg]because it's initializing the next, not-yet-counted slot beforemp->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)
Verified
__os_detach)memp001/003+mvcc001passThis is the 3rd real bug surfaced by the coverage program (after #67 heap-regression and #77 upstream
__db_set_lastpgnooff-by-one).