/modrestart used to be implemented as a normal command and could crash
when used remotely because it would reload m_encap, which was on the
call stack at the time. This was fixed in 41390bfe5f. However,
/modreload has exactly the same problem, so I'm giving it the
same treatment.
Incidentally: This bug was first discovered in ircd-seven, where the
`/mod*` commands themselves live in the core, so m_encap was the only way
the crash could happen (and it didn't most of the time, because m_encap
would only be moved if you got unlucky). But `/mod*` are in modules in
charybdis, so /modrestart would have unloaded the code it was in the
middle of executing. With that in mind, I'm not sure how it ever
appeared to work.
Reloading modules sends CAP DEL followed by an immediate CAP NEW:
:staberinde.local CAP * DEL :account-tag
:staberinde.local CAP * NEW :account-tag
This isn't very nice. /modrestart is particularly bad. In order to avoid
doing this, we remember the capability set at the beginning of module
operations, compare that with the set afterwards, and report only the
differences with CAP {DEL,NEW}.
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 db05a36210
Reported-by: mniip (Freenode)
[ircd/match.c:316]: (error) Shifting a negative value is undefined behaviour
[librb/src/patricia.c:55]: (error) Shifting a negative value is undefined behaviour
[modules/m_alias.c:64]: (portability) '(void*)message' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[modules/m_time.c:111]: (warning) %u in format string (no. 9) requires 'unsigned int' but the argument type is 'signed int'.
[modules/m_time.c:111]: (warning) %u in format string (no. 10) requires 'unsigned int' but the argument type is 'signed int'.
[librb/src/dictionary.c:819]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[librb/src/radixtree.c:1080]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[ircd/s_user.c:351] -> [ircd/s_user.c:357]: (warning) Either the condition '0!=source_p' is redundant or there is possible null pointer dereference: source_p.
[extensions/ip_cloaking_3.0.c:109]: (warning, inconclusive) The buffer 'buf' may not be null-terminated after the call to strncpy().
[ircd/chmode.c:256]: (style) Clarify calculation precedence for '&' and '?'.
[modules/m_help.c:100]: (style) Clarify calculation precedence for '&' and '?'.
[modules/m_knock.c:169]: (style) Clarify calculation precedence for '&' and '?'.
[modules/m_stats.c:628]: (style) Clarify calculation precedence for '&' and '?'.
[modules/m_stats.c:727]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:601]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:704]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:739]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:763]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:768]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:774]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:781]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:786]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:791]: (style) Clarify calculation precedence for '&' and '?'.
[librb/src/radixtree.c:804]: (style) Clarify calculation precedence for '&' and '?'.
[ircd/wsproc.c:372]: (style) Unused variable: len
[modules/core/m_modules.c:382]: (style) Unused variable: i
[modules/m_stats.c:741]: (style) Unused variable: amsg
[ircd/authproc.c:390]: (style) Unused variable: iter
[ircd/authproc.c:391]: (style) Unused variable: client_p
There's no reason to really have these in the main ircd anymore, static
modules are dead and aren't coming back.
To ensure people don't do something hopelessly retarded, this is a core
module.