Use-after-free / memory safety

PoC (conceptual reproduction for test/harness): Prerequisites: - Build Redis with ASAN and run the unit tests for kvstore/dict (the repo’s test harness). - Ensure you’re using the affected 8.6.2 baseline (or older in 8.6.x). Goal: Reproduce a use-after-free in kvstoreIncrementallyRehash by creating a dict that starts rehashing, then deletes the last element such that the dict is freed while still in rehashing, leading kvstoreIncrementallyRehash to access freed memory. The fix ensures rehashingCompleted is called when the dict is released/emptied. Conceptual steps that lead to the vulnerability (pre-fix behavior): 1) Put a dictionary into a rehashing state (rehashingStarted) via the kvs/dict lifecycle during an operation that triggers two-phase unlink for a db entry in a rehashing table. 2) Delete the last remaining element from the dict while it is still in rehashing. This can free the dict in kvstoreDictTwoPhaseUnlinkFree without unregistering it from the kvs->rehashing list. 3) A periodic serverCron path calls kvstoreIncrementallyRehash and uses the freed dict pointer, causing a crash. Fixed behavior (after patch): - dictRelease and dictEmpty now call dict.type.rehashingCompleted(d) if the dict is rehashing, ensuring rehashingCompleted is always paired with rehashingStarted. - This prevents kvstoreIncrementallyRehash from touching freed memory. Minimal unit-test style reproduction (as included in the patch): - Create or obtain a kvs instance where a dict is put into rehashing (rehash path starts). - Force deletion of an entry that leads to the last element removal while rehashing is still in progress. - Call kvstoreIncrementallyRehash in a draining loop and ensure the dict is no longer in rehashing state and no crash occurs. - The test includes a step to drain the rehash loop and verify the dict is no longer present: while (kvstoreIncrementiallyRehash(kvs2, 1000)) {} dict *d = kvstoreGetDict(kvs2, didx); assert(d == NULL); This PoC mirrors the fix’s intent: ensuring rehashingCompleted is always called before/when a dict is released during rehashing, thereby preventing use-after-free in the rehashing path.

diff --git a/src/dict.c b/src/dict.c index 6e9b13150bf..d04f9e1231a 100644 --- a/src/dict.c +++ b/src/dict.c @@ -706,6 +706,10 @@ int _dictClear(dict *d, int htidx, void(callback)(dict*)) { /* Clear & Release the hash table */ void dictRelease(dict *d) { + /* Someone may be monitoring a dict that started rehashing, before + * destroying the dict fake completion. */ + if (dictIsRehashing(d) && d->type->rehashingCompleted) + d->type->rehashingCompleted(d); _dictClear(d,0,NULL); _dictClear(d,1,NULL); zfree(d); @@ -1588,6 +1592,10 @@ void *dictFindPositionForInsert(dict *d, const void *key, dictEntry **existing) } void dictEmpty(dict *d, void(callback)(dict*)) { + /* Someone may be monitoring a dict that started rehashing, before + * destroying the dict fake completion. */ + if (dictIsRehashing(d) && d->type->rehashingCompleted) + d->type->rehashingCompleted(d); _dictClear(d,0,callback); _dictClear(d,1,callback); d->rehashidx = -1; diff --git a/src/kvstore.c b/src/kvstore.c index 505f92957fa..62b799dddc1 100644 --- a/src/kvstore.c +++ b/src/kvstore.c @@ -957,6 +957,9 @@ int kvstoreTest(int argc, char **argv, int flags) { } kvstoreIteratorRelease(kvs_it); + /* Make sure the dict was removed from the rehashing list. */ + while (kvstoreIncrementallyRehash(kvs2, 1000)) {} + dict *d = kvstoreGetDict(kvs2, didx); assert(d == NULL); assert(kvstoreDictSize(kvs2, didx) == 0);