From f7a3a38227958558985f5f7109c3ed1dbf26832c Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Fri, 27 Sep 2019 17:20:40 +0100 Subject: [PATCH] PR libstdc++/91910 fix data race in Debug Mode destructors Fix data race when _Safe_iterator_base::_M_detach() runs concurrently with the _Safe_container_base destructor. PR libstdc++/91910 * src/c++11/debug.cc (_Safe_iterator_base::_M_detach()): Load pointer atomically and lock the mutex before accessing the sequence. (_Safe_local_iterator_base::_M_detach()): Likewise. (_Safe_iterator_base::_M_reset()): Clear _M_sequence atomically. From-SVN: r276184 --- libstdc++-v3/ChangeLog | 8 ++++++++ libstdc++-v3/src/c++11/debug.cc | 21 ++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 4b8bc7ac50f..0b19f29ddbe 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,11 @@ +2019-09-27 Jonathan Wakely + + PR libstdc++/91910 + * src/c++11/debug.cc (_Safe_iterator_base::_M_detach()): Load pointer + atomically and lock the mutex before accessing the sequence. + (_Safe_local_iterator_base::_M_detach()): Likewise. + (_Safe_iterator_base::_M_reset()): Clear _M_sequence atomically. + 2019-09-26 Jonathan Wakely * include/debug/array (to_array): Define for debug mode. diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index f5a49992efa..efd1a9e0254 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -381,10 +381,17 @@ namespace __gnu_debug _Safe_iterator_base:: _M_detach() { - if (_M_sequence) + // This function can run concurrently with the sequence destructor, + // so there is a TOCTTOU race here: the sequence could be destroyed + // after we check that _M_sequence is not null. Use the pointer value + // to acquire the mutex (rather than via _M_sequence->_M_get_mutex()). + // If the sequence destructor runs between loading the pointer and + // locking the mutex, it will detach this iterator and set _M_sequence + // to null, and then _M_detach_single() will do nothing. + if (auto seq = __atomic_load_n(&_M_sequence, __ATOMIC_ACQUIRE)) { - _M_sequence->_M_detach(this); - _M_reset(); + __gnu_cxx::__scoped_lock sentry(get_safe_base_mutex(seq)); + _M_detach_single(); } } @@ -403,7 +410,7 @@ namespace __gnu_debug _Safe_iterator_base:: _M_reset() throw () { - _M_sequence = 0; + __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE); _M_version = 0; _M_prior = 0; _M_next = 0; @@ -466,10 +473,10 @@ namespace __gnu_debug _Safe_local_iterator_base:: _M_detach() { - if (_M_sequence) + if (auto seq = __atomic_load_n(&_M_sequence, __ATOMIC_ACQUIRE)) { - _M_get_container()->_M_detach_local(this); - _M_reset(); + __gnu_cxx::__scoped_lock sentry(get_safe_base_mutex(seq)); + _M_detach_single(); } } -- 2.30.2