From 08d714d0e5d59f1c1ca37cff1724849220fb6daf Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Wed, 8 Jan 2025 09:57:23 +0800
Subject: [PATCH] Fix crash due to cron argv release (#13725)

Introduced by https://github.com/redis/redis/issues/13521

If the client argv was released due to a timeout before sending the
complete command, `argv_len` will be reset to 0.
When argv is parsed again and resized, requesting a length of 0 may
result in argv being NULL, then leading to a crash.

And fix a bug that `argv_len` is not updated correctly in
`replaceClientCommandVector()`.

---------

Co-authored-by: ShooterIT <wangyuancode@163.com>
Co-authored-by: meiravgri <109056284+meiravgri@users.noreply.github.com>
---
 src/networking.c        |  3 ++-
 src/server.c            |  5 +++--
 tests/unit/protocol.tcl | 11 +++++++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/networking.c b/src/networking.c
index 94303e1b9..fd9905e2f 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -2505,6 +2505,7 @@ int processMultibulkBuffer(client *c) {
         } else {
             /* Check if we have space in argv, grow if needed */
             if (c->argc >= c->argv_len) {
+                serverAssert(c->argv_len); /* Ensure argv is not freed while the client is in the mid of parsing command. */
                 c->argv_len = min(c->argv_len < INT_MAX/2 ? c->argv_len*2 : INT_MAX, c->argc+c->multibulklen);
                 c->argv = zrealloc(c->argv, sizeof(robj*)*c->argv_len);
             }
@@ -3990,7 +3991,7 @@ void replaceClientCommandVector(client *c, int argc, robj **argv) {
     retainOriginalCommandVector(c);
     freeClientArgv(c);
     c->argv = argv;
-    c->argc = argc;
+    c->argc = c->argv_len = argc;
     c->argv_len_sum = 0;
     for (j = 0; j < c->argc; j++)
         if (c->argv[j])
diff --git a/src/server.c b/src/server.c
index 7bc39c411..a4c8f1152 100644
--- a/src/server.c
+++ b/src/server.c
@@ -788,8 +788,9 @@ int clientsCronResizeQueryBuffer(client *c) {
 
 /* If the client has been idle for too long, free the client's arguments. */
 int clientsCronFreeArgvIfIdle(client *c) {
-    /* If the arguments have already been freed or are still in use, exit ASAP. */
-    if (!c->argv || c->argc) return 0;
+    /* If the client is in the middle of parsing a command, or if argv is in use
+     * (e.g. parsed in the IO thread but not yet executed, or blocked), exit ASAP. */
+    if (!c->argv || c->multibulklen || c->argc) return 0;
     time_t idletime = server.unixtime - c->lastinteraction;
     if (idletime > 2) {
         c->argv_len = 0;
diff --git a/tests/unit/protocol.tcl b/tests/unit/protocol.tcl
index a98b124b7..8a6aeed4a 100644
--- a/tests/unit/protocol.tcl
+++ b/tests/unit/protocol.tcl
@@ -310,3 +310,14 @@ start_server {tags {"regression"}} {
         $rd close
     }
 }
+
+start_server {tags {"regression"}} {
+    test "Regression for a crash with cron release of client arguments" {
+        r write "*3\r\n"
+        r flush
+        after 3000 ;# wait for c->argv to be released due to timeout
+        r write "\$3\r\nSET\r\n\$3\r\nkey\r\n\$1\r\n0\r\n"
+        r flush
+        r read
+    } {OK}
+}