From 5fa0f0d171889da0e5c3aa7904386df99006aeb1 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 19 Mar 2018 11:46:21 +0000 Subject: [PATCH] Problem: global random init/deinit breaks existing applications Solution: restrict it only to the original issue #2632, Tweetnacl on *NIX when using /dev/urandom, ie: without the new Linux getrandom() syscall. Existing applications might use atexit to register cleanup functions (like CZMQ does), and the current change as-is imposes an ordering that did not exist before - the context MUST be created BEFORE registering the cleanup with atexit. This is a backward incompatible change that is reported to cause aborts in some applications. Although libsodium's documentation says that its initialisation APIs is not thread-safe, nobody has ever reported an issue with it, so avoiding the global init/deinit in the libsodium case is the less risky option we have. Tweetnacl users on Windows and on Linux with getrandom (glibc 2.25 and Linux kernel 3.17) are not affected by the original issue. Fixes #2991 --- src/random.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/random.cpp b/src/random.cpp index 91f654ea9f..859111d4b5 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -86,6 +86,13 @@ uint32_t zmq::generate_random () // order fiasco, this is done using function-local statics, if the // compiler implementation supports thread-safe initialization of those. // Otherwise, we fall back to global statics. +// HOWEVER, this initialisation code adds a race condition when an +// application uses atexit or similar methods for cleanup callbacks. +// In that case, a strict ordering is imposed whereas the contexts MUST +// be initialised BEFORE registering the cleanup with atexit. CZMQ is an +// example. Hence we make the choice to restrict this global transition +// mechanism ONLY to Tweenacl + *NIX (when using /dev/urandom) as it is +// the less risky option. // TODO if there is some other user of libsodium besides libzmq, this must // be synchronized by the application. This should probably also be @@ -105,18 +112,16 @@ uint32_t zmq::generate_random () #endif #if !ZMQ_HAVE_THREADSAFE_STATIC_LOCAL_INIT \ - && (defined(ZMQ_USE_LIBSODIUM) \ - || (defined(ZMQ_USE_TWEETNACL) && !defined(ZMQ_HAVE_WINDOWS) \ - && !defined(ZMQ_HAVE_GETRANDOM))) + && (defined(ZMQ_USE_TWEETNACL) && !defined(ZMQ_HAVE_WINDOWS) \ + && !defined(ZMQ_HAVE_GETRANDOM)) static unsigned int random_refcount = 0; static zmq::mutex_t random_sync; #endif static void manage_random (bool init) { -#if defined(ZMQ_USE_LIBSODIUM) \ - || (defined(ZMQ_USE_TWEETNACL) && !defined(ZMQ_HAVE_WINDOWS) \ - && !defined(ZMQ_HAVE_GETRANDOM)) +#if defined(ZMQ_USE_TWEETNACL) && !defined(ZMQ_HAVE_WINDOWS) \ + && !defined(ZMQ_HAVE_GETRANDOM) #if ZMQ_HAVE_THREADSAFE_STATIC_LOCAL_INIT static int random_refcount = 0; @@ -140,6 +145,14 @@ static void manage_random (bool init) randombytes_close (); } } + +#elif defined(ZMQ_USE_LIBSODIUM) + if (init) { + int rc = sodium_init (); + zmq_assert (rc != -1); + } else { + randombytes_close (); + } #endif }