From 7ef0383905b75f070a0d9cbfc1bd47d773a44d3d Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Sat, 4 Oct 2025 11:02:35 +0200 Subject: [PATCH 1/2] Fix #14404 don't add loadmodule when from config --- src/config.c | 3 +++ src/module.c | 16 +++++++++------- src/server.h | 5 +++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/config.c b/src/config.c index 2606d065027..973a31a5100 100644 --- a/src/config.c +++ b/src/config.c @@ -381,6 +381,7 @@ void queueLoadModule(sds path, sds *argv, int argc) { loadmod->argv = argc ? zmalloc(sizeof(robj*)*argc) : NULL; loadmod->path = sdsnew(path); loadmod->argc = argc; + loadmod->conf = 1; for (i = 0; i < argc; i++) { loadmod->argv[i] = createRawStringObject(argv[i],sdslen(argv[i])); } @@ -1589,6 +1590,8 @@ void rewriteConfigLoadmoduleOption(struct rewriteConfigState *state) { struct RedisModule *module = dictGetVal(de); /* Internal modules doesn't have path and are not part of the configuration file */ if (sdslen(module->loadmod->path) == 0) continue; + /* ignore when loaded from config */ + if (module->loadmod->conf) continue; line = sdsnew("loadmodule "); line = sdscatsds(line, module->loadmod->path); diff --git a/src/module.c b/src/module.c index ab8cafb191a..2d3b8a4eb3e 100644 --- a/src/module.c +++ b/src/module.c @@ -13126,7 +13126,7 @@ int VectorSets_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc); /* Load internal data types that bundled as modules */ void moduleLoadInternalModules(void) { #ifdef INCLUDE_VEC_SETS - int retval = moduleOnLoad((int (*)(void *, void **, int)) VectorSets_OnLoad, NULL, NULL, NULL, 0, 0); + int retval = moduleOnLoad((int (*)(void *, void **, int)) VectorSets_OnLoad, NULL, NULL, NULL, 0, 0, 1); serverAssert(retval == C_OK); #endif } @@ -13147,7 +13147,7 @@ void moduleLoadFromQueue(void) { listRewind(server.loadmodule_queue,&li); while((ln = listNext(&li))) { struct moduleLoadQueueEntry *loadmod = ln->value; - if (moduleLoad(loadmod->path,(void **)loadmod->argv,loadmod->argc, 0) + if (moduleLoad(loadmod->path,(void **)loadmod->argv, loadmod->argc, 0, loadmod->conf) == C_ERR) { serverLog(LL_WARNING, @@ -13336,7 +13336,7 @@ void moduleUnregisterCleanup(RedisModule *module) { /* Load a module by path and initialize it. On success C_OK is returned, otherwise * C_ERR is returned. */ -int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex) { +int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex, int is_config) { int (*onload)(void *, void **, int); void *handle; @@ -13363,12 +13363,12 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa return C_ERR; } - return moduleOnLoad(onload, path, handle, module_argv, module_argc, is_loadex); + return moduleOnLoad(onload, path, handle, module_argv, module_argc, is_loadex, is_config); } /* Load a module by its 'onload' callback and initialize it. On success C_OK is returned, otherwise * C_ERR is returned. */ -int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex) { +int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int is_config) { RedisModuleCtx ctx; moduleCreateContext(&ctx, NULL, REDISMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ if (onload((void*)&ctx,module_argv,module_argc) == REDISMODULE_ERR) { @@ -13392,6 +13392,8 @@ int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *ha ctx.module->loadmod->path = sdsnew(path); ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj*)*module_argc) : NULL; ctx.module->loadmod->argc = module_argc; + ctx.module->loadmod->conf = is_config; + for (int i = 0; i < module_argc; i++) { ctx.module->loadmod->argv[i] = module_argv[i]; incrRefCount(ctx.module->loadmod->argv[i]); @@ -14706,7 +14708,7 @@ NULL argv = &c->argv[3]; } - if (moduleLoad(c->argv[2]->ptr,(void **)argv,argc, 0) == C_OK) + if (moduleLoad(c->argv[2]->ptr,(void **)argv,argc, 0, 0) == C_OK) addReply(c,shared.ok); else addReplyError(c, @@ -14722,7 +14724,7 @@ NULL /* If this is a loadex command we want to populate server.module_configs_queue with * sds NAME VALUE pairs. We also want to increment argv to just after ARGS, if supplied. */ if (parseLoadexArguments((RedisModuleString ***) &argv, &argc) == REDISMODULE_OK && - moduleLoad(c->argv[2]->ptr, (void **)argv, argc, 1) == C_OK) + moduleLoad(c->argv[2]->ptr, (void **)argv, argc, 1, 0) == C_OK) addReply(c,shared.ok); else { dictEmpty(server.module_configs_queue, NULL); diff --git a/src/server.h b/src/server.h index a5cf8a73fe0..d724f09e193 100644 --- a/src/server.h +++ b/src/server.h @@ -1704,6 +1704,7 @@ struct saveparam { struct moduleLoadQueueEntry { sds path; + int conf; int argc; robj **argv; }; @@ -3095,8 +3096,8 @@ void populateCommandLegacyRangeSpec(struct redisCommand *c); void moduleInitModulesSystem(void); void moduleInitModulesSystemLast(void); void modulesCron(void); -int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex); -int moduleLoad(const char *path, void **argv, int argc, int is_loadex); +int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int is_config); +int moduleLoad(const char *path, void **argv, int argc, int is_loadex, int is_config); int moduleUnload(sds name, const char **errmsg, int forced_unload); void moduleLoadInternalModules(void); void moduleLoadFromQueue(void); From 92d7e87872db47edc07e6349fcdadacde9a1d8ef Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Mon, 6 Oct 2025 02:52:16 +0200 Subject: [PATCH 2/2] only protect loadmodule from include files --- src/config.c | 9 +++++++-- src/module.c | 10 +++++----- src/server.h | 6 +++--- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/config.c b/src/config.c index 973a31a5100..ea8b8dab55b 100644 --- a/src/config.c +++ b/src/config.c @@ -373,6 +373,9 @@ void resetServerSaveParams(void) { server.saveparamslen = 0; } +/* support detecting include vs main config file */ +static int reading_include_file = 0; + void queueLoadModule(sds path, sds *argv, int argc) { int i; struct moduleLoadQueueEntry *loadmod; @@ -381,7 +384,7 @@ void queueLoadModule(sds path, sds *argv, int argc) { loadmod->argv = argc ? zmalloc(sizeof(robj*)*argc) : NULL; loadmod->path = sdsnew(path); loadmod->argc = argc; - loadmod->conf = 1; + loadmod->from_include = reading_include_file;; for (i = 0; i < argc; i++) { loadmod->argv[i] = createRawStringObject(argv[i],sdslen(argv[i])); } @@ -541,7 +544,9 @@ void loadServerConfigFromString(char *config) { /* Execute config directives */ if (!strcasecmp(argv[0],"include") && argc == 2) { + reading_include_file = 1; loadServerConfig(argv[1], 0, NULL); + reading_include_file = 0; } else if (!strcasecmp(argv[0],"rename-command") && argc == 3) { struct redisCommand *cmd = lookupCommandBySds(argv[1]); int retval; @@ -1591,7 +1596,7 @@ void rewriteConfigLoadmoduleOption(struct rewriteConfigState *state) { /* Internal modules doesn't have path and are not part of the configuration file */ if (sdslen(module->loadmod->path) == 0) continue; /* ignore when loaded from config */ - if (module->loadmod->conf) continue; + if (module->loadmod->from_include) continue; line = sdsnew("loadmodule "); line = sdscatsds(line, module->loadmod->path); diff --git a/src/module.c b/src/module.c index 2d3b8a4eb3e..d8c887fcdbf 100644 --- a/src/module.c +++ b/src/module.c @@ -13147,7 +13147,7 @@ void moduleLoadFromQueue(void) { listRewind(server.loadmodule_queue,&li); while((ln = listNext(&li))) { struct moduleLoadQueueEntry *loadmod = ln->value; - if (moduleLoad(loadmod->path,(void **)loadmod->argv, loadmod->argc, 0, loadmod->conf) + if (moduleLoad(loadmod->path,(void **)loadmod->argv, loadmod->argc, 0, loadmod->from_include) == C_ERR) { serverLog(LL_WARNING, @@ -13336,7 +13336,7 @@ void moduleUnregisterCleanup(RedisModule *module) { /* Load a module by path and initialize it. On success C_OK is returned, otherwise * C_ERR is returned. */ -int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex, int is_config) { +int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex, int from_include) { int (*onload)(void *, void **, int); void *handle; @@ -13363,12 +13363,12 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa return C_ERR; } - return moduleOnLoad(onload, path, handle, module_argv, module_argc, is_loadex, is_config); + return moduleOnLoad(onload, path, handle, module_argv, module_argc, is_loadex, from_include); } /* Load a module by its 'onload' callback and initialize it. On success C_OK is returned, otherwise * C_ERR is returned. */ -int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int is_config) { +int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int from_include) { RedisModuleCtx ctx; moduleCreateContext(&ctx, NULL, REDISMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ if (onload((void*)&ctx,module_argv,module_argc) == REDISMODULE_ERR) { @@ -13392,7 +13392,7 @@ int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *ha ctx.module->loadmod->path = sdsnew(path); ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj*)*module_argc) : NULL; ctx.module->loadmod->argc = module_argc; - ctx.module->loadmod->conf = is_config; + ctx.module->loadmod->from_include = from_include; for (int i = 0; i < module_argc; i++) { ctx.module->loadmod->argv[i] = module_argv[i]; diff --git a/src/server.h b/src/server.h index d724f09e193..f3c836c9edc 100644 --- a/src/server.h +++ b/src/server.h @@ -1704,7 +1704,7 @@ struct saveparam { struct moduleLoadQueueEntry { sds path; - int conf; + int from_include; int argc; robj **argv; }; @@ -3096,8 +396,8 @@ void populateCommandLegacyRangeSpec(struct redisCommand *c); void moduleInitModulesSystem(void); void moduleInitModulesSystemLast(void); void modulesCron(void); -int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int is_config); -int moduleLoad(const char *path, void **argv, int argc, int is_loadex, int is_config); +int moduleOnLoad(int (*onload)(void *, void **, int), const char *path, void *handle, void **module_argv, int module_argc, int is_loadex, int from_include); +int moduleLoad(const char *path, void **argv, int argc, int is_loadex, int from_include); int moduleUnload(sds name, const char **errmsg, int forced_unload); void moduleLoadInternalModules(void); void moduleLoadFromQueue(void);