From 1c0f376dd35aaf97140dfb7b551a52e176729941 Mon Sep 17 00:00:00 2001 From: Pekka Vuorela Date: Mon, 17 Sep 2018 17:10:58 +0300 Subject: Fix unsafe signal handling in messageserver Qt code cannot be called from signal handlers so old version wasn't safe. Funny thing MessageServer already had proper handling, but it was overridden for sighup in main.cpp. Change-Id: I9ab55f943148b5cd62bb01eec0157e2500b57ba2 Reviewed-by: Christopher Adams Reviewed-by: Matthew Vogt --- src/tools/messageserver/main.cpp | 30 +------------ src/tools/messageserver/messageserver.cpp | 74 +++++++++++++++++++++++++++---- src/tools/messageserver/messageserver.h | 8 ++++ 3 files changed, 75 insertions(+), 37 deletions(-) diff --git a/src/tools/messageserver/main.cpp b/src/tools/messageserver/main.cpp index 0076e711..9c107629 100644 --- a/src/tools/messageserver/main.cpp +++ b/src/tools/messageserver/main.cpp @@ -36,30 +36,11 @@ #include #include #include -#include -#include + #ifdef USE_HTML_PARSER #include #endif -#if !defined(NO_SHUTDOWN_SIGNAL_HANDLING) && defined(Q_OS_UNIX) - -static void shutdown(int n) -{ - qMailLog(Messaging) << "Received signal" << n << ", shutting down."; - QCoreApplication::exit(); -} -#endif - -#if defined(Q_OS_UNIX) - -static void recreateLoggers(int n) -{ - qMailLoggersRecreate("QtProject", "Messageserver", "Msgsrv"); - qMailLog(Messaging) << "Received signal" << n << ", logs recreated."; -} -#endif - int main(int argc, char** argv) { #ifdef USE_HTML_PARSER @@ -78,15 +59,6 @@ int main(int argc, char** argv) MessageServer server; -#if !defined(NO_SHUTDOWN_SIGNAL_HANDLING) && defined(Q_OS_UNIX) - signal(SIGINT, shutdown); - signal(SIGTERM, shutdown); -#endif - -#if defined(Q_OS_UNIX) - signal(SIGHUP,recreateLoggers); -#endif - int exitCode = app.exec(); return exitCode; diff --git a/src/tools/messageserver/messageserver.cpp b/src/tools/messageserver/messageserver.cpp index e6adea2e..b9cae752 100644 --- a/src/tools/messageserver/messageserver.cpp +++ b/src/tools/messageserver/messageserver.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -55,6 +56,8 @@ extern "C" { #if defined(Q_OS_UNIX) #include int MessageServer::sighupFd[2]; +int MessageServer::sigtermFd[2]; +int MessageServer::sigintFd[2]; #endif MessageServer::MessageServer(QObject *parent) @@ -69,22 +72,41 @@ MessageServer::MessageServer(QObject *parent) new QCopServer(this); #if defined(Q_OS_UNIX) - // SIGHUP handler. We use the trick described here: http://doc.trolltech.com/4.7-snapshot/unix-signals.html + // Unix signal handlers. We use the trick described here: http://doc.qt.io/qt-5/unix-signals.html // Looks shocking but the trick has certain reasons stated in Steven's book: http://cr.yp.to/docs/selfpipe.html // Use a socket and notifier because signal handlers can't call Qt code if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sighupFd)) qFatal("Couldn't create HUP socketpair"); + if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sigtermFd)) + qFatal("Couldn't create TERM socketpair"); + if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sigintFd)) + qFatal("Couldn't create TERM socketpair"); + snHup = new QSocketNotifier(sighupFd[1], QSocketNotifier::Read, this); connect(snHup, SIGNAL(activated(int)), this, SLOT(handleSigHup())); - - struct sigaction hup; - hup.sa_handler = MessageServer::hupSignalHandler; - sigemptyset(&hup.sa_mask); - hup.sa_flags = 0; - hup.sa_flags |= SA_RESTART; - if (sigaction(SIGHUP, &hup, 0) > 0) + snTerm = new QSocketNotifier(sigtermFd[1], QSocketNotifier::Read, this); + connect(snTerm, SIGNAL(activated(int)), this, SLOT(handleSigTerm())); + snInt = new QSocketNotifier(sigintFd[1], QSocketNotifier::Read, this); + connect(snInt, SIGNAL(activated(int)), this, SLOT(handleSigInt())); + + struct sigaction action; + action.sa_handler = MessageServer::hupSignalHandler; + sigemptyset(&action.sa_mask); + action.sa_flags = 0; + action.sa_flags |= SA_RESTART; + + if (sigaction(SIGHUP, &action, 0) > 0) qFatal("Couldn't register HUP handler"); + + action.sa_handler = MessageServer::termSignalHandler; + if (sigaction(SIGTERM, &action, 0) > 0) + qFatal("Couldn't register TERM handler"); + + action.sa_handler = MessageServer::intSignalHandler; + if (sigaction(SIGINT, &action, 0) > 0) + qFatal("Couldn't register INT handler"); + #endif // defined(Q_OS_UNIX) QMailMessageCountMap::iterator it = messageCounts.begin(), end = messageCounts.end(); @@ -521,4 +543,40 @@ void MessageServer::handleSigHup() snHup->setEnabled(true); } +void MessageServer::termSignalHandler(int) +{ + char a = 1; + ::write(sigtermFd[0], &a, sizeof(a)); +} + +void MessageServer::handleSigTerm() +{ + snTerm->setEnabled(false); + char tmp; + ::read(sigtermFd[1], &tmp, sizeof(tmp)); + + qMailLog(Messaging) << "Received SIGTERM, shutting down."; + QCoreApplication::exit(); + + snTerm->setEnabled(true); +} + +void MessageServer::intSignalHandler(int) +{ + char a = 1; + ::write(sigintFd[0], &a, sizeof(a)); +} + +void MessageServer::handleSigInt() +{ + snInt->setEnabled(false); + char tmp; + ::read(sigintFd[1], &tmp, sizeof(tmp)); + + qMailLog(Messaging) << "Received SIGINT, shutting down."; + QCoreApplication::exit(); + + snInt->setEnabled(true); +} + #endif // defined(Q_OS_UNIX) diff --git a/src/tools/messageserver/messageserver.h b/src/tools/messageserver/messageserver.h index 6ab03507..185de679 100644 --- a/src/tools/messageserver/messageserver.h +++ b/src/tools/messageserver/messageserver.h @@ -61,6 +61,8 @@ public: #if defined(Q_OS_UNIX) static void hupSignalHandler(int unused); // Unix SIGHUP signal handler + static void termSignalHandler(int unused); + static void intSignalHandler(int unused); #endif signals: @@ -68,6 +70,8 @@ signals: #if defined(Q_OS_UNIX) public slots: void handleSigHup(); // Qt signal handler for UNIX SIGHUP signal. + void handleSigTerm(); + void handleSigInt(); #endif private slots: @@ -105,6 +109,10 @@ private: #if defined(Q_OS_UNIX) static int sighupFd[2]; QSocketNotifier *snHup; + static int sigtermFd[2]; + QSocketNotifier *snTerm; + static int sigintFd[2]; + QSocketNotifier *snInt; #endif }; -- cgit v1.2.3