Skip to content

Commit

Permalink
Problem: fd leak in tweetnacl with one ctx per thread
Browse files Browse the repository at this point in the history
Solution: check for thread-local storage support in the compiler and
if available use it for the global file descriptor variable used to
open /dev/urandom in the tweetnacl code.
if TLS is not available fallback to wrapping randombytes and
randombytes_close into critical sections using a pthread mutex, to
forcefully serialize open and close.
Fixes zeromq#2632
  • Loading branch information
bluca committed Jul 27, 2017
1 parent 2991e6f commit a0a8c00
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 0 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ if (NOT CMAKE_CROSSCOMPILING)
zmq_check_tcp_keepalive ()
zmq_check_tcp_tipc ()
zmq_check_pthread_setname ()
zmq_check_thread_local_storage ()
endif ()

if ( CMAKE_SYSTEM_NAME MATCHES "Linux"
Expand Down
22 changes: 22 additions & 0 deletions acinclude.m4
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,28 @@ int main (int argc, char *argv [])
AS_IF([test "x$libzmq_cv_tcp_keepalive" = "xyes"], [$1], [$2])
}])

dnl ################################################################################
dnl # LIBZMQ_CHECK_THREAD_LOCAL_STORAGE([action-if-found], [action-if-not-found]) #
dnl # Checks if thread-local storage is supported #
dnl ################################################################################
AC_DEFUN([LIBZMQ_CHECK_THREAD_LOCAL_STORAGE], [{
AC_CACHE_CHECK([whether thread-local storage is supported], [libzmq_cv_thread_local_storage],
[AC_TRY_RUN([/* thread-local storage test */
static __thread int i = 0;
int main (int argc, char *argv [])
{
i = 1;
}
],
[libzmq_cv_thread_local_storage="yes"],
[libzmq_cv_thread_local_storage="no"],
[libzmq_cv_thread_local_storage="not during cross-compile"]
)]
)
AS_IF([test "x$libzmq_cv_thread_local_storage" = "xyes"], [$1], [$2])
}])

dnl ################################################################################
dnl # LIBZMQ_CHECK_POLLER_KQUEUE([action-if-found], [action-if-not-found]) #
dnl # Checks kqueue polling system #
Expand Down
15 changes: 15 additions & 0 deletions builds/cmake/Modules/ZMQSourceRunChecks.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,18 @@ int main(int argc, char *argv [])
ZMQ_HAVE_PTHREAD_SET_NAME)
set(CMAKE_REQUIRED_FLAGS ${SAVE_CMAKE_REQUIRED_FLAGS})
endmacro()


macro(zmq_check_thread_local_storage)
message(STATUS "Checking whether thread-local storage is supported")
check_c_source_runs(
"
static __thread int i = 0;
int main (int argc, char *argv [])
{
i = 1;
}
"
ZMQ_HAVE_THREAD_LOCAL_STORAGE)
endmacro()
6 changes: 6 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,12 @@ LIBZMQ_CHECK_TCP_KEEPALIVE([
[Whether TCP_KEEPALIVE is supported.])
])

LIBZMQ_CHECK_THREAD_LOCAL_STORAGE([
AC_DEFINE([ZMQ_HAVE_THREAD_LOCAL_STORAGE],
[1],
[Whether thread-local storage is supported.])
])

AM_CONDITIONAL(HAVE_FORK, test "x$ac_cv_func_fork" = "xyes")

if test "x$cross_compiling" = "xyes"; then
Expand Down
29 changes: 29 additions & 0 deletions src/tweetnacl.c
Original file line number Diff line number Diff line change
Expand Up @@ -905,11 +905,28 @@ int randombytes_close(void)
#include <fcntl.h>
#include <unistd.h>

#include <stdio.h>
#ifdef ZMQ_HAVE_THREAD_LOCAL_STORAGE
static __thread int fd = -1;
#else
/*
When different threads have their own context the file descriptor
variable is shared and is subject to race conditions.
thread-local storage has been available in most compilers since the
early 2000s, but just in case use a critical section as a fallback.
*/
#include <pthread.h>
static int fd = -1;
static pthread_mutex_t fd_lock = PTHREAD_MUTEX_INITIALIZER;
#endif

void randombytes (unsigned char *x,unsigned long long xlen)
{
int i;
#ifndef ZMQ_HAVE_THREAD_LOCAL_STORAGE
int rc = pthread_mutex_lock (&fd_lock);
assert (rc == 0);
#endif
if (fd == -1) {
for (;;) {
fd = open("/dev/urandom",O_RDONLY);
Expand All @@ -931,15 +948,27 @@ void randombytes (unsigned char *x,unsigned long long xlen)
x += i;
xlen -= i;
}
#ifndef ZMQ_HAVE_THREAD_LOCAL_STORAGE
rc = pthread_mutex_unlock (&fd_lock);
assert (rc == 0);
#endif
}

int randombytes_close (void)
{
#ifndef ZMQ_HAVE_THREAD_LOCAL_STORAGE
int pt_rc = pthread_mutex_lock (&fd_lock);
assert (pt_rc == 0);
#endif
int rc = -1;
if (fd != -1 && close(fd) == 0) {
fd = -1;
rc = 0;
}
#ifndef ZMQ_HAVE_THREAD_LOCAL_STORAGE
pt_rc = pthread_mutex_unlock (&fd_lock);
assert (pt_rc == 0);
#endif
return rc;
}

Expand Down

0 comments on commit a0a8c00

Please sign in to comment.