gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

Fix memory leak when RM_Call's RUN_AS_USER fails (#12158)

Browse Source 
previously the argv wasn't freed so would leak.  not a common case, but should be handled.

Solution: move RUN_AS_USER setup and error exit to the right place.
this way, when we do `goto cleanup` (instead of return) it'll automatically do the right thing (including autoMemoryAdd)
Removed the user argument from moduleAllocTempClient (reverted to the state before 6e993a5)

Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
Shaya Potter 2023-05-23 14:29:28 +03:00 committed by GitHub
parent 006ab26c37
commit 71e6abe423
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 20 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/module.c
View File

@ -620,7 +620,7 @@ void *RM_PoolAlloc(RedisModuleCtx *ctx, size_t bytes) {
* Helpers for modules API implementation
* -------------------------------------------------------------------------- */
client *moduleAllocTempClient(user *user) {
client *moduleAllocTempClient(void) {
client *c = NULL;
if (moduleTempClientCount > 0) {
@ -630,10 +630,8 @@ client *moduleAllocTempClient(user *user) {
} else {
c = createClient(NULL);
c->flags |= CLIENT_MODULE;
c->user = NULL; /* Root user */
}
c->user = user;
return c;
}
@ -879,7 +877,7 @@ void moduleCreateContext(RedisModuleCtx *out_ctx, RedisModule *module, int ctx_f
out_ctx->module = module;
out_ctx->flags = ctx_flags;
if (ctx_flags & REDISMODULE_CTX_TEMP_CLIENT)
out_ctx->client = moduleAllocTempClient(NULL);
out_ctx->client = moduleAllocTempClient();
else if (ctx_flags & REDISMODULE_CTX_NEW_CLIENT)
out_ctx->client = createClient(NULL);
@ -6215,20 +6213,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
error_as_call_replies = flags & REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS;
va_end(ap);
user *user = NULL;
if (flags & REDISMODULE_ARGV_RUN_AS_USER) {
user = ctx->user ? ctx->user->user : ctx->client->user;
if (!user) {
errno = ENOTSUP;
if (error_as_call_replies) {
sds msg = sdsnew("cannot run as user, no user directly attached to context or context's client");
reply = callReplyCreateError(msg, ctx);
}
return reply;
}
}
c = moduleAllocTempClient(user);
c = moduleAllocTempClient();
if (!(flags & REDISMODULE_ARGV_ALLOW_BLOCK)) {
/* We do not want to allow block, the module do not expect it */
@ -6248,6 +6233,20 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
}
if (ctx->module) ctx->module->in_call++;
user *user = NULL;
if (flags & REDISMODULE_ARGV_RUN_AS_USER) {
user = ctx->user ? ctx->user->user : ctx->client->user;
if (!user) {
errno = ENOTSUP;
if (error_as_call_replies) {
sds msg = sdsnew("cannot run as user, no user directly attached to context or context's client");
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
c->user = user;
}
/* We handle the above format error only when the client is setup so that
* we can free it normally. */
if (argv == NULL) {
@ -7680,8 +7679,8 @@ RedisModuleBlockedClient *moduleBlockClient(RedisModuleCtx *ctx, RedisModuleCmdF
bc->disconnect_callback = NULL; /* Set by RM_SetDisconnectCallback() */
bc->free_privdata = free_privdata;
bc->privdata = privdata;
bc->reply_client = moduleAllocTempClient(NULL);
bc->thread_safe_ctx_client = moduleAllocTempClient(NULL);
bc->reply_client = moduleAllocTempClient();
bc->thread_safe_ctx_client = moduleAllocTempClient();
if (bc->client)
bc->reply_client->resp = bc->client->resp;
bc->dbid = c->db->id;