From 85b46d57bcc40b8053dafe5d0ebb4b0bb611b484 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 17 Jan 2014 12:04:53 -0500 Subject: [PATCH 1/2] Check spawn_func() return value If we don't, we can wind up with a wedged cpuworker, and write to it for ages and ages. Found by skruffy. This was a bug in 2dda97e8fd898757, a.k.a. svn revision 402. It's been there since we have been using cpuworkers. --- changes/bug4345 | 6 ++++++ src/or/cpuworker.c | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 changes/bug4345 diff --git a/changes/bug4345 b/changes/bug4345 new file mode 100644 index 0000000000..4975eea1ea --- /dev/null +++ b/changes/bug4345 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Check return code on spawn_func() in cpuworker code, so that we don't + think we've spawned a nonworking cpuworker and write junk to it + forever. Fix for bug 4345; bugfix on all released Tor versions. + Found by "skruffy". + diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index ecf0d2035d..2f9f527e97 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -528,7 +528,12 @@ spawn_cpuworker(void) tor_assert(SOCKET_OK(fdarray[1])); fd = fdarray[0]; - spawn_func(cpuworker_main, (void*)fdarray); + if (spawn_func(cpuworker_main, (void*)fdarray) < 0) { + tor_close_socket(fdarray[0]); + tor_close_socket(fdarray[1]); + tor_free(fdarray); + return -1; + } log_debug(LD_OR,"just spawned a cpu worker."); #ifndef TOR_IS_MULTITHREADED tor_close_socket(fdarray[1]); /* don't need the worker's side of the pipe */ From 1ebdaf5788cb7969ee3f853ede4e6e3364343fc0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 7 Feb 2014 13:13:15 -0500 Subject: [PATCH 2/2] More hacking around spawn_func issues This time, we use a pthread_attr to make sure that if pthread_create succeeds, the thread is successfully detached. This probably isn't the big thing going on with 4345, since it'd be a bit weird for pthread_detach to be failing. But it's worth getting it right. --- changes/bug4345 | 6 +++++- src/common/compat.c | 16 +++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/changes/bug4345 b/changes/bug4345 index 4975eea1ea..5e650fc340 100644 --- a/changes/bug4345 +++ b/changes/bug4345 @@ -1,6 +1,10 @@ o Minor bugfixes: - Check return code on spawn_func() in cpuworker code, so that we don't think we've spawned a nonworking cpuworker and write junk to it - forever. Fix for bug 4345; bugfix on all released Tor versions. + forever. Fix related to bug 4345; bugfix on all released Tor versions. Found by "skruffy". + - Use a pthread_attr to make sure that spawn_func() cannot return + an error while at the same time launching a thread. Fix related + to bug 4345; bugfix on all released Tor versions. Reported by + "cypherpunks". diff --git a/src/common/compat.c b/src/common/compat.c index d88c5f92de..8a3655e194 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -2248,6 +2248,12 @@ tor_pthread_helper_fn(void *_data) func(arg); return NULL; } +/** + * A pthread attribute to make threads start detached. + */ +static pthread_attr_t attr_detached; +/** True iff we've called tor_threads_init() */ +static int threads_initialized = 0; #endif /** Minimalist interface to run a void function in the background. On @@ -2271,12 +2277,12 @@ spawn_func(void (*func)(void *), void *data) #elif defined(USE_PTHREADS) pthread_t thread; tor_pthread_data_t *d; + if (PREDICT_UNLIKELY(!threads_initialized)) + tor_threads_init(); d = tor_malloc(sizeof(tor_pthread_data_t)); d->data = data; d->func = func; - if (pthread_create(&thread,NULL,tor_pthread_helper_fn,d)) - return -1; - if (pthread_detach(thread)) + if (pthread_create(&thread,&attr_detached,tor_pthread_helper_fn,d)) return -1; return 0; #else @@ -2633,8 +2639,6 @@ tor_get_thread_id(void) * "reentrant" mutexes (i.e., once we can re-lock if we're already holding * them.) */ static pthread_mutexattr_t attr_reentrant; -/** True iff we've called tor_threads_init() */ -static int threads_initialized = 0; /** Initialize mutex so it can be locked. Every mutex must be set * up with tor_mutex_init() or tor_mutex_new(); not both. */ void @@ -2778,6 +2782,8 @@ tor_threads_init(void) if (!threads_initialized) { pthread_mutexattr_init(&attr_reentrant); pthread_mutexattr_settype(&attr_reentrant, PTHREAD_MUTEX_RECURSIVE); + tor_assert(0==pthread_attr_init(&attr_detached)); + tor_assert(0==pthread_attr_setdetachstate(&attr_detached, 1)); threads_initialized = 1; set_main_thread(); }