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 @@ -362,6 +362,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])); } @@ -1570,6 +1571,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 @@ -12333,7 +12333,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 } @@ -12354,7 +12354,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, @@ -12543,7 +12543,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; @@ -12570,12 +12570,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) { @@ -12599,6 +12599,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]); @@ -13910,7 +13912,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, @@ -13926,7 +13928,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 @@ -1499,6 +1499,7 @@ struct saveparam { struct moduleLoadQueueEntry { sds path; + int conf; int argc; robj **argv; }; @@ -2745,8 +2746,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 @@ -354,6 +354,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; @@ -362,7 +365,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])); } @@ -522,7 +525,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; @@ -1572,7 +1577,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 @@ -12354,7 +12354,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, @@ -12543,7 +12543,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; @@ -12570,12 +12570,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) { @@ -12599,7 +12599,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 @@ -1499,7 +1499,7 @@ struct saveparam { struct moduleLoadQueueEntry { sds path; - int conf; + int from_include; int argc; robj **argv; }; @@ -2746,8 +2746,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);