From d339fe70adc335dcf8b0e03cbb52bcd0263afcd5 Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Sun, 7 Sep 2025 17:07:48 +0800
Subject: [PATCH] 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.
---
 src/defrag.c |  3 ++-
 src/dict.c   | 24 ++++++++++++++++++++++++
 src/dict.h   |  1 +
 src/eval.c   | 14 +++++++++-----
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/defrag.c b/src/defrag.c
index c1e2334e9d..91c573c0cf 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -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);
         }
diff --git a/src/dict.c b/src/dict.c
index 0ea6353379..8dbe835951 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -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. 
diff --git a/src/dict.h b/src/dict.h
index 698088ab96..27b37233cb 100644
--- a/src/dict.h
+++ b/src/dict.h
@@ -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);
diff --git a/src/eval.c b/src/eval.c
index 91bda9b1c3..55c9718fb4 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -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) {