From f3217e260549a53f8961cd7873ac4dd4a2dc5e82 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson@est.tech>
Date: Tue, 10 Sep 2024 08:26:14 +0200
Subject: [PATCH] Cleanup of cluster free functions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Remove special handling when freeing the nodes dict. This is now
prevented by the newly added flag VALKEYCLUSTER_FLAG_DISCONNECTING.

Scrub the valkeyClusterContext before freeing to help us detect use-after-free.
This is similar to how valkeyContext is freed.

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
---
 src/cluster.c | 47 +++++++++--------------------------------------
 1 file changed, 9 insertions(+), 38 deletions(-)

diff --git a/src/cluster.c b/src/cluster.c
index b24cd21..2f7234f 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -1364,7 +1364,6 @@ valkeyClusterContext *valkeyClusterContextInit(void) {
 }
 
 void valkeyClusterFree(valkeyClusterContext *cc) {
-
     if (cc == NULL)
         return;
 
@@ -1373,45 +1372,21 @@ void valkeyClusterFree(valkeyClusterContext *cc) {
                            cc->event_privdata);
     }
 
-    if (cc->connect_timeout) {
-        vk_free(cc->connect_timeout);
-        cc->connect_timeout = NULL;
-    }
-
-    if (cc->command_timeout) {
-        vk_free(cc->command_timeout);
-        cc->command_timeout = NULL;
-    }
-
-    if (cc->table != NULL) {
-        vk_free(cc->table);
-        cc->table = NULL;
-    }
+    vk_free(cc->connect_timeout);
+    vk_free(cc->command_timeout);
+    vk_free(cc->username);
+    vk_free(cc->password);
+    vk_free(cc->table);
 
     if (cc->nodes != NULL) {
-        /* Clear cc->nodes before releasing the dict since the release procedure
-           might access cc->nodes. When a node and its valkey context are freed
-           all pending callbacks are executed. Clearing cc->nodes prevents a pending
-           slotmap update command callback to trigger additional slotmap updates. */
-        dict *nodes = cc->nodes;
-        cc->nodes = NULL;
-        dictRelease(nodes);
+        dictRelease(cc->nodes);
     }
 
     if (cc->requests != NULL) {
         listRelease(cc->requests);
     }
 
-    if (cc->username != NULL) {
-        vk_free(cc->username);
-        cc->username = NULL;
-    }
-
-    if (cc->password != NULL) {
-        vk_free(cc->password);
-        cc->password = NULL;
-    }
-
+    memset(cc, 0xff, sizeof(*cc));
     vk_free(cc);
 }
 
@@ -3673,15 +3648,11 @@ void valkeyClusterAsyncDisconnect(valkeyClusterAsyncContext *acc) {
 }
 
 void valkeyClusterAsyncFree(valkeyClusterAsyncContext *acc) {
-    valkeyClusterContext *cc;
-
-    if (acc == NULL) {
+    if (acc == NULL)
         return;
-    }
 
-    cc = acc->cc;
+    valkeyClusterContext *cc = acc->cc;
     cc->flags |= VALKEYCLUSTER_FLAG_DISCONNECTING;
-
     valkeyClusterFree(cc);
 
     vk_free(acc);