October 2019 - tor-commits - lists.torproject.org

[tor/master] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 34f3d8d407d67a617aef54ac78326c1c1e34936b Merge: e5b9eefb0 d4dde249a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:59 2019 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2

1 0

[tor/release-0.4.1] Merge remote-tracking branch 'tor-github/pr/1346' into maint-0.4.1
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit d4dde249a04c768381767f67a1a82425bc70baaa Merge: 6965798a1 d1eab0583 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:50 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1346' into maint-0.4.1 changes/bug31736 | 3 +++ src/app/config/config.c | 6 +++++- src/feature/relay/router.c | 4 ++++ src/lib/crypt_ops/crypto_openssl_mgt.c | 4 ++++ src/lib/lock/compat_mutex.c | 10 +++++++++- src/lib/lock/compat_mutex_pthreads.c | 16 +++++++++++++++- src/lib/thread/compat_threads.c | 10 +++++++++- src/lib/thread/threads.h | 12 +++++++++++- 8 files changed, 60 insertions(+), 5 deletions(-)

1 0

[tor/release-0.4.2] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 34f3d8d407d67a617aef54ac78326c1c1e34936b Merge: e5b9eefb0 d4dde249a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:59 2019 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2

1 0

[tor/release-0.4.1] Merge branch 'maint-0.4.1' into release-0.4.1
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit f954ae6883fcca3522e05fa24fb4b542ee62929c Merge: 8bef3a76a d4dde249a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:59 2019 -0400 Merge branch 'maint-0.4.1' into release-0.4.1 changes/bug31736 | 3 +++ src/app/config/config.c | 6 +++++- src/feature/relay/router.c | 4 ++++ src/lib/crypt_ops/crypto_openssl_mgt.c | 4 ++++ src/lib/lock/compat_mutex.c | 10 +++++++++- src/lib/lock/compat_mutex_pthreads.c | 16 +++++++++++++++- src/lib/thread/compat_threads.c | 10 +++++++++- src/lib/thread/threads.h | 12 +++++++++++- 8 files changed, 60 insertions(+), 5 deletions(-)

1 0

[tor/release-0.4.2] Merge remote-tracking branch 'tor-github/pr/1346' into maint-0.4.1
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit d4dde249a04c768381767f67a1a82425bc70baaa Merge: 6965798a1 d1eab0583 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:50 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1346' into maint-0.4.1 changes/bug31736 | 3 +++ src/app/config/config.c | 6 +++++- src/feature/relay/router.c | 4 ++++ src/lib/crypt_ops/crypto_openssl_mgt.c | 4 ++++ src/lib/lock/compat_mutex.c | 10 +++++++++- src/lib/lock/compat_mutex_pthreads.c | 16 +++++++++++++++- src/lib/thread/compat_threads.c | 10 +++++++++- src/lib/thread/threads.h | 12 +++++++++++- 8 files changed, 60 insertions(+), 5 deletions(-)

1 0

[tor/release-0.4.2] Merge branch 'maint-0.4.2' into release-0.4.2
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 6f3ee65f5340b47ebb6e215e5cc1dbd6e013778b Merge: 11b1e1715 34f3d8d40 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:59 2019 -0400 Merge branch 'maint-0.4.2' into release-0.4.2

1 0

[tor/master] Merge branch 'maint-0.4.2'
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 3939bb82f0f222550385dd583355f57e7ec6f9ea Merge: da59dee5d 34f3d8d40 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:11:59 2019 -0400 Merge branch 'maint-0.4.2'

1 0

[tor/release-0.4.1] lock: Avoid some undefined behaviour when freeing mutexes.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit d1eab05834566f998721d3a16107767885711c57 Author: teor <teor(a)torproject.org> Date: Fri Sep 20 11:27:05 2019 +1000 lock: Avoid some undefined behaviour when freeing mutexes. Fixes bug 31736; bugfix on 0.0.7. --- changes/bug31736 | 3 +++ src/app/config/config.c | 6 +++++- src/feature/relay/router.c | 4 ++++ src/lib/crypt_ops/crypto_openssl_mgt.c | 4 ++++ src/lib/lock/compat_mutex.c | 10 +++++++++- src/lib/lock/compat_mutex_pthreads.c | 16 +++++++++++++++- src/lib/thread/compat_threads.c | 10 +++++++++- src/lib/thread/threads.h | 12 +++++++++++- 8 files changed, 60 insertions(+), 5 deletions(-) diff --git a/changes/bug31736 b/changes/bug31736 new file mode 100644 index 000000000..beb09e506 --- /dev/null +++ b/changes/bug31736 @@ -0,0 +1,3 @@ + o Minor bugfixes (multithreading): + - Avoid some undefined behaviour when freeing mutexes. + Fixes bug 31736; bugfix on 0.0.7. diff --git a/src/app/config/config.c b/src/app/config/config.c index 0b1b758d9..2416da2b2 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -1187,7 +1187,11 @@ init_protocol_warning_severity_level(void) static void cleanup_protocol_warning_severity_level(void) { - atomic_counter_destroy(&protocol_warning_severity_level); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ + atomic_counter_destroy(&protocol_warning_severity_level); } /** List of default directory authorities */ diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index dad2c6a50..af47e79d2 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -3072,6 +3072,10 @@ router_free_all(void) crypto_pk_free(server_identitykey); crypto_pk_free(client_identitykey); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ tor_mutex_free(key_lock); routerinfo_free(desc_routerinfo); extrainfo_free(desc_extrainfo); diff --git a/src/lib/crypt_ops/crypto_openssl_mgt.c b/src/lib/crypt_ops/crypto_openssl_mgt.c index c97815f9a..a245ef381 100644 --- a/src/lib/crypt_ops/crypto_openssl_mgt.c +++ b/src/lib/crypt_ops/crypto_openssl_mgt.c @@ -176,6 +176,10 @@ crypto_openssl_free_all(void) tor_free(crypto_openssl_version_str); tor_free(crypto_openssl_header_version_str); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ #ifndef NEW_THREAD_API if (n_openssl_mutexes_) { int n = n_openssl_mutexes_; diff --git a/src/lib/lock/compat_mutex.c b/src/lib/lock/compat_mutex.c index 4ad592971..670bd0174 100644 --- a/src/lib/lock/compat_mutex.c +++ b/src/lib/lock/compat_mutex.c @@ -29,7 +29,15 @@ tor_mutex_new_nonrecursive(void) tor_mutex_init_nonrecursive(m); return m; } -/** Release all storage and system resources held by m. */ +/** Release all storage and system resources held by m. + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void tor_mutex_free_(tor_mutex_t *m) { diff --git a/src/lib/lock/compat_mutex_pthreads.c b/src/lib/lock/compat_mutex_pthreads.c index ee5f520cd..f82ad9f0e 100644 --- a/src/lib/lock/compat_mutex_pthreads.c +++ b/src/lib/lock/compat_mutex_pthreads.c @@ -88,12 +88,26 @@ tor_mutex_release(tor_mutex_t *m) } /** Clean up the mutex m so that it no longer uses any system * resources. Does not free m. This function must only be called on - * mutexes from tor_mutex_init(). */ + * mutexes from tor_mutex_init(). + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void tor_mutex_uninit(tor_mutex_t *m) { int err; raw_assert(m); + /* If the mutex is already locked, wait until after it is unlocked to destroy + * it. Locking and releasing the mutex makes undefined behaviour less likely, + * but does not prevent it. Another thread can lock the mutex between release + * and destroy. */ + tor_mutex_acquire(m); + tor_mutex_release(m); err = pthread_mutex_destroy(&m->mutex); if (PREDICT_UNLIKELY(err)) { // LCOV_EXCL_START diff --git a/src/lib/thread/compat_threads.c b/src/lib/thread/compat_threads.c index 94ab021c5..16cece612 100644 --- a/src/lib/thread/compat_threads.c +++ b/src/lib/thread/compat_threads.c @@ -65,7 +65,15 @@ atomic_counter_init(atomic_counter_t *counter) memset(counter, 0, sizeof(*counter)); tor_mutex_init_nonrecursive(&counter->mutex); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void atomic_counter_destroy(atomic_counter_t *counter) { diff --git a/src/lib/thread/threads.h b/src/lib/thread/threads.h index ecf60641b..de3da6a58 100644 --- a/src/lib/thread/threads.h +++ b/src/lib/thread/threads.h @@ -131,7 +131,17 @@ atomic_counter_init(atomic_counter_t *counter) { atomic_init(&counter->val, 0); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * This usage note applies to the compat_threads implementation of + * atomic_counter_destroy(): + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ static inline void atomic_counter_destroy(atomic_counter_t *counter) {

1 0

[tor/maint-0.3.5] Appveyor: Avoid spurious errors in Appveyor CI builds
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit b186418792fe1283855acf2f143661b7c6d7254e Author: teor <teor(a)torproject.org> Date: Mon Sep 30 15:14:21 2019 +1000 Appveyor: Avoid spurious errors in Appveyor CI builds When Appveyor fails before the install step, some of the finish step's functions were not defined. Fixes bug 31884; bugfix on 0.3.4.2-alpha. --- .appveyor.yml | 12 ++++++++++++ changes/bug31884 | 3 +++ 2 files changed, 15 insertions(+) diff --git a/.appveyor.yml b/.appveyor.yml index 9913bf3bd..54456aa25 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -100,6 +100,18 @@ test_script: on_finish: - ps: >- + <# if we failed before install:, these functions won't be defined #> + Function Execute-Command ($commandPath) + { + & $commandPath $args 2>&1 + if ( $LastExitCode -ne 0 ) { + $host.SetShouldExit( $LastExitCode ) + } + } + Function Execute-Bash () + { + Execute-Command 'c:\msys64\usr\bin\bash' '-e' '-c' $args + } if ($env:compiler -eq "mingw") { <# use the MSYS2 user binaries to archive failures #> $oldpath = ${env:Path} -split ';' diff --git a/changes/bug31884 b/changes/bug31884 new file mode 100644 index 000000000..ddb6c50d7 --- /dev/null +++ b/changes/bug31884 @@ -0,0 +1,3 @@ + o Minor bugfixes (Appveyor CI): + - Avoid spurious errors when Appveyor CI fails before the install step. + Fixes bug 31884; bugfix on 0.3.4.2-alpha.

1 0

[tor/maint-0.3.5] Merge remote-tracking branch 'tor-github/pr/1376' into maint-0.3.5
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 21c3488af714ade6f07251194165d92f77f06c31 Merge: bed439013 b18641879 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:10:08 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1376' into maint-0.3.5 .appveyor.yml | 12 ++++++++++++ changes/bug31884 | 3 +++ 2 files changed, 15 insertions(+)

1 0