Fix defrag issues for pubsub and lua (#14330)

This PR fixes two defrag issues.

1. Fix a use-after-free issue caused by updating dictionary keys after a
pubsub channel is reallocated.
This issue was introduced by https://github.com/redis/redis/pull/13058

1. Fix potential use-after-free for lua during AOF loading with defrag
This issue was introduced by https://github.com/redis/redis/issues/13058
   This fix follows https://github.com/redis/redis/pull/14319
This PR updates the LuaScript LRU list before script execution to
prevent accessing a potentially invalidated pointer after long-running
scripts.
This commit is contained in:
debing.sun 2025-09-07 17:07:48 +08:00 committed by GitHub
parent 64546d2009
commit d339fe70ad
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 36 additions and 6 deletions

View file

@ -1049,7 +1049,8 @@ void defragPubsubScanCallback(void *privdata, const dictEntry *de, dictEntryLink
while((clientde = dictNext(&di)) != NULL) {
client *c = dictGetKey(clientde);
dict *client_channels = ctx->getPubSubChannels(c);
dictEntry *pubsub_channel = dictFind(client_channels, newchannel);
uint64_t hash = dictGetHash(client_channels, newchannel);
dictEntry *pubsub_channel = dictFindByHashAndPtr(client_channels, channel, hash);
serverAssert(pubsub_channel);
dictSetKey(ctx->getPubSubChannels(c), pubsub_channel, newchannel);
}

View file

@ -809,6 +809,30 @@ dictEntry *dictFind(dict *d, const void *key)
return (link) ? *link : NULL;
}
/* Finds the dictEntry using pointer and pre-calculated hash.
* oldkey is a dead pointer and should not be accessed.
* the hash value should be provided using dictGetHash.
* no string / key comparison is performed.
* return value is a pointer to the dictEntry if found, or NULL if not found. */
dictEntry *dictFindByHashAndPtr(dict *d, const void *oldptr, const uint64_t hash) {
dictEntry *he;
unsigned long idx, table;
if (dictSize(d) == 0) return NULL; /* dict is empty */
for (table = 0; table <= 1; table++) {
idx = hash & DICTHT_SIZE_MASK(d->ht_size_exp[table]);
if (table == 0 && (long)idx < d->rehashidx) continue;
he = d->ht_table[table][idx];
while(he) {
if (oldptr == dictGetKey(he))
return he;
he = dictGetNext(he);
}
if (!dictIsRehashing(d)) return NULL;
}
return NULL;
}
/* Find a key and return its dictEntryLink reference. Otherwise, return NULL
*
* A dictEntryLink pointer being used to find preceding dictEntry of searched item.

View file

@ -226,6 +226,7 @@ dictEntryLink dictTwoPhaseUnlinkFind(dict *d, const void *key, int *table_index)
void dictTwoPhaseUnlinkFree(dict *d, dictEntryLink llink, int table_index);
void dictRelease(dict *d);
dictEntry * dictFind(dict *d, const void *key);
dictEntry *dictFindByHashAndPtr(dict *d, const void *oldptr, const uint64_t hash);
int dictShrinkIfNeeded(dict *d);
int dictExpandIfNeeded(dict *d);
void *dictGetKey(const dictEntry *de);

View file

@ -611,17 +611,21 @@ void evalGenericCommand(client *c, int evalsha) {
rctx.flags |= SCRIPT_EVAL_MODE; /* mark the current run as EVAL (as opposed to FCALL) so we'll
get appropriate error messages and logs */
luaCallFunction(&rctx, lua, c->argv+3, numkeys, c->argv+3+numkeys, c->argc-3-numkeys, ldb.active);
lua_pop(lua,1); /* Remove the error handler. */
scriptResetRun(&rctx);
luaGC(lua, &gc_count);
if (l->node) {
/* Quick removal and re-insertion after the script is called to
* maintain the LRU list. */
listUnlinkNode(lctx.lua_scripts_lru_list, l->node);
listLinkNodeTail(lctx.lua_scripts_lru_list, l->node);
}
luaCallFunction(&rctx, lua, c->argv+3, numkeys, c->argv+3+numkeys, c->argc-3-numkeys, ldb.active);
lua_pop(lua,1); /* Remove the error handler. */
scriptResetRun(&rctx);
luaGC(lua, &gc_count);
/* We can no longer touch 'l' here, as it may have been reallocated by activedefrag
* during AOF loading of long-running scripts. This issue is not with newly generated
* AOF files, in which scripts propagate effects rather than scripts. */
}
void evalCommand(client *c) {