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>
This commit is contained in:
debing.sun 2025-01-08 09:57:23 +08:00 committed by GitHub
parent 4a12291765
commit 08d714d0e5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 16 additions and 3 deletions

View File

@ -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])

View File

@ -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;

View File

@ -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}
}