charon-systemd: Don't use atexit() to deinitialize the daemon
authorTobias Brunner <tobias@strongswan.org>
Fri, 28 Sep 2018 17:55:52 +0000 (19:55 +0200)
committerTobias Brunner <tobias@strongswan.org>
Wed, 21 Nov 2018 13:31:49 +0000 (14:31 +0100)
This is because OpenSSL 1.1 started to use atexit()-handlers of its own
to clean up.  Since the plugin is loaded and initialized after libcharon,
OpenSSL's cleanup functions ran before the daemon was properly
deinitialized (i.e. worker threads were still running and OpenSSL might
still be used during the deinit).  So several of OpenSSL's internal
structures were already destroyed when libcharon_deinit() was eventually
called via our own atexit()-handler.

The observed behavior was that the daemon couldn't be terminated properly
anymore for some test scenarios (only three TNC scenarios were affected
actually).  When the daemon tried to send the DELETE for the established
IKE_SA during its termination it got stuck in OpenSSL's RNG_WEAK
implementation (used to allocate random padding), which apparently tries
to acquire an rwlock that was already destroyed.  The main thread then
just busy-waited indefinitely on the lock, i.e. until systemd killed
it eventually after a rather long timeout.

We'll probably have to apply similar changes to other apps/scripts that
load plugins and currently use atexit() to clean up.  Although some
scripts (e.g. dh_speed or hash_burn) are not affected because they
register the deinitialization after loading the plugins.

src/charon-systemd/charon-systemd.c

index d06c269..7d4465e 100644 (file)
@@ -322,6 +322,7 @@ int main(int argc, char *argv[])
 {
        struct sigaction action;
        struct utsname utsname;
+       int status = SS_RC_INITIALIZATION_FAILED;
 
        dbg = dbg_stderr;
 
@@ -345,16 +346,15 @@ int main(int argc, char *argv[])
                sd_notifyf(0, "STATUS=integrity check of charon-systemd failed");
                return SS_RC_INITIALIZATION_FAILED;
        }
-       atexit(libcharon_deinit);
        if (!libcharon_init())
        {
                sd_notifyf(0, "STATUS=libcharon initialization failed");
-               return SS_RC_INITIALIZATION_FAILED;
+               goto error;
        }
        if (!lookup_uid_gid())
        {
                sd_notifyf(0, "STATUS=unknown uid/gid");
-               return SS_RC_INITIALIZATION_FAILED;
+               goto error;
        }
        /* we registered the journal logger as custom logger, which gets its
         * settings from <ns>.customlog.journal, let it fallback to <ns>.journal */
@@ -370,14 +370,14 @@ int main(int argc, char *argv[])
                        lib->settings->get_str(lib->settings, "%s.load", PLUGINS, lib->ns)))
        {
                sd_notifyf(0, "STATUS=charon initialization failed");
-               return SS_RC_INITIALIZATION_FAILED;
+               goto error;
        }
        lib->plugins->status(lib->plugins, LEVEL_CTRL);
 
        if (!lib->caps->drop(lib->caps))
        {
                sd_notifyf(0, "STATUS=dropping capabilities failed");
-               return SS_RC_INITIALIZATION_FAILED;
+               goto error;
        }
 
        /* add handler for SEGV and ILL,
@@ -401,5 +401,9 @@ int main(int argc, char *argv[])
        sd_notifyf(0, "STATUS=charon-systemd running, strongSwan %s, %s %s, %s",
                           VERSION, utsname.sysname, utsname.release, utsname.machine);
 
-       return run();
+       status = run();
+
+error:
+       libcharon_deinit();
+       return status;
 }