From 41390bfe5fa3895b8deec57f987b71f90ada1f7d Mon Sep 17 00:00:00 2001
From: Aaron Jones <aaronmdjones@gmail.com>
Date: Wed, 28 Dec 2016 22:08:14 +0000
Subject: [PATCH] When a remote MODRESTART command is received, it will pass
 through the ENCAP module. The ms_encap function is responsible for
 dispatching the command handler and then the modules will eventually be
 reloaded.

However, if the ENCAP module is reloaded to a different address, the
stack now contains the address of a function that no longer exists.

Also, in this version of the IRCd, the module restarting functionality
was located in a function that is itself located in a module, so things
will also go badly if that module is reloaded to a different address,
too.

Return immediately from the command handler and have the event loop
call the function responsible for reloading the modules instead.

c.f. release/3.5 commit db05a3621058

Reported-by: mniip (Freenode)
---
 ircd/modules.c           | 35 ++++++++++++++++++++++++++++++++
 modules/core/m_modules.c | 43 ++++++++++++----------------------------
 2 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/ircd/modules.c b/ircd/modules.c
index 1d969af7..ffa08077 100644
--- a/ircd/modules.c
+++ b/ircd/modules.c
@@ -668,3 +668,38 @@ load_a_module(const char *path, bool warn, int origin, bool core)
 	rb_free(mod_displayname);
 	return true;
 }
+
+void
+modules_do_restart(void *unused)
+{
+	unsigned int modnum = 0;
+	rb_dlink_node *ptr, *nptr;
+
+	RB_DLINK_FOREACH_SAFE(ptr, nptr, module_list.head)
+	{
+		struct module *mod = ptr->data;
+		if(!unload_one_module(mod->name, false))
+		{
+			ilog(L_MAIN, "Module Restart: %s was not unloaded %s",
+			     mod->name,
+			     mod->core? "(core module)" : "");
+
+			if(!mod->core)
+				sendto_realops_snomask(SNO_GENERAL, L_NETWIDE,
+				                       "Module Restart: %s failed to unload",
+				                       mod->name);
+			continue;
+		}
+
+		modnum++;
+	}
+
+	load_all_modules(false);
+	load_core_modules(false);
+	rehash(false);
+
+	sendto_realops_snomask(SNO_GENERAL, L_NETWIDE,
+			     "Module Restart: %u modules unloaded, %lu modules loaded",
+			     modnum, rb_dlink_list_length(&module_list));
+	ilog(L_MAIN, "Module Restart: %u modules unloaded, %lu modules loaded", modnum, rb_dlink_list_length(&module_list));
+}
diff --git a/modules/core/m_modules.c b/modules/core/m_modules.c
index 67944bcc..312d26d1 100644
--- a/modules/core/m_modules.c
+++ b/modules/core/m_modules.c
@@ -54,6 +54,8 @@ static void do_modreload(struct Client *, const char *);
 static void do_modlist(struct Client *, const char *);
 static void do_modrestart(struct Client *);
 
+extern void modules_do_restart(void *); /* end of ircd/modules.c */
+
 struct Message modload_msgtab = {
 	"MODLOAD", 0, 0, 0, 0,
 	{mg_unreg, mg_not_oper, mg_ignore, mg_ignore, {me_modload, 2}, {mo_modload, 2}}
@@ -341,38 +343,19 @@ do_modreload(struct Client *source_p, const char *module)
 static void
 do_modrestart(struct Client *source_p)
 {
-	unsigned int modnum = 0;
-	rb_dlink_node *ptr, *nptr;
-
 	sendto_one_notice(source_p, ":Reloading all modules");
 
-	RB_DLINK_FOREACH_SAFE(ptr, nptr, module_list.head)
-	{
-		struct module *mod = ptr->data;
-		if(!unload_one_module(mod->name, false))
-		{
-			ilog(L_MAIN, "Module Restart: %s was not unloaded %s",
-			     mod->name,
-			     mod->core? "(core module)" : "");
-
-			if(!mod->core)
-				sendto_realops_snomask(SNO_GENERAL, L_NETWIDE,
-				                       "Module Restart: %s failed to unload",
-				                       mod->name);
-			continue;
-		}
-
-		modnum++;
-	}
-
-	load_all_modules(false);
-	load_core_modules(false);
-	rehash(false);
-
-	sendto_realops_snomask(SNO_GENERAL, L_NETWIDE,
-			     "Module Restart: %u modules unloaded, %lu modules loaded",
-			     modnum, rb_dlink_list_length(&module_list));
-	ilog(L_MAIN, "Module Restart: %u modules unloaded, %lu modules loaded", modnum, rb_dlink_list_length(&module_list));
+	/*
+	 * If a remote MODRESTART is received, m_encap.so will be reloaded,
+	 * but ms_encap is in the call stack (it indirectly calls this
+	 * function). Also, this function is itself in a module.
+	 *
+	 * This will go horribly wrong if either module is reloaded to a
+	 * different address.
+	 *
+	 * So, defer the restart to the event loop and return now.
+	 */
+	rb_event_addonce("modules_do_restart", modules_do_restart, NULL, 1);
 }
 
 static void