Fix module loadex command crash due to invalid config (#13653)

Fix to https://github.com/redis/redis/issues/13650

providing an invalid config to a module with datatype crashes when redis
tries to unload the module due to the invalid config

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
This commit is contained in:
nafraf
2025-04-22 13:49:12 +03:00
committed by YaacovHazan
co-authored by debing.sun
parent 8e72b42a9a
commit 6e819e188b
4 changed files with 23 additions and 6 deletions
+8 -5
View File
@@ -12262,7 +12262,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa
}
if (post_load_err) {
moduleUnload(ctx.module->name, NULL);
serverAssert(moduleUnload(ctx.module->name, NULL, 1) == C_OK);
moduleFreeContext(&ctx);
return C_ERR;
}
@@ -12278,14 +12278,17 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa
/* Unload the module registered with the specified name. On success
* C_OK is returned, otherwise C_ERR is returned and errmsg is set
* with an appropriate message. */
int moduleUnload(sds name, const char **errmsg) {
* with an appropriate message.
* Only forcefully unload this module, passing forced_unload != 0,
* if it is certain that it has not yet been in use (e.g., immediate
* unload on failed load). */
int moduleUnload(sds name, const char **errmsg, int forced_unload) {
struct RedisModule *module = dictFetchValue(modules,name);
if (module == NULL) {
*errmsg = "no such module with that name";
return C_ERR;
} else if (listLength(module->types)) {
} else if (listLength(module->types) && !forced_unload) {
*errmsg = "the module exports one or more module-side data "
"types, can't unload";
return C_ERR;
@@ -13078,7 +13081,7 @@ NULL
} else if (!strcasecmp(subcmd,"unload") && c->argc == 3) {
const char *errmsg = NULL;
if (moduleUnload(c->argv[2]->ptr, &errmsg) == C_OK)
if (moduleUnload(c->argv[2]->ptr, &errmsg, 0) == C_OK)
addReply(c,shared.ok);
else {
if (errmsg == NULL) errmsg = "operation not possible.";
+1 -1
View File
@@ -2469,7 +2469,7 @@ void moduleInitModulesSystem(void);
void moduleInitModulesSystemLast(void);
void modulesCron(void);
int moduleLoad(const char *path, void **argv, int argc, int is_loadex);
int moduleUnload(sds name, const char **errmsg);
int moduleUnload(sds name, const char **errmsg, int forced_unload);
void moduleLoadFromQueue(void);
int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result);
int moduleGetCommandChannelsViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result);
+9
View File
@@ -312,3 +312,12 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_OK;
}
int RedisModule_OnUnload(RedisModuleCtx *ctx) {
REDISMODULE_NOT_USED(ctx);
if (datatype) {
RedisModule_Free(datatype);
datatype = NULL;
}
return REDISMODULE_OK;
}
+5
View File
@@ -1,6 +1,11 @@
set testmodule [file normalize tests/modules/datatype.so]
start_server {tags {"modules"}} {
test {DataType: test loadex with invalid config} {
catch { r module loadex $testmodule CONFIG invalid_config 1 } e
assert_match {*ERR Error loading the extension*} $e
}
r module load $testmodule
test {DataType: Test module is sane, GET/SET work.} {