[libcxx-commits] [libcxx] r355550 - [libc++] Fix use-after-free when building with _LIBCPP_DEBUG=1
Thomas Anderson via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Mar 6 13:10:08 PST 2019
Author: thomasanderson
Date: Wed Mar 6 13:10:08 2019
New Revision: 355550
URL: http://llvm.org/viewvc/llvm-project?rev=355550&view=rev
Log:
[libc++] Fix use-after-free when building with _LIBCPP_DEBUG=1
The issue is the following code:
__cn1->__add(*__ip);
(*__ip)->__c_ = __cn1;
`__ip` points into the array of iterators for container `__cn2`. This code adds
the iterator to the array of iterators for `__cn1`, and updates the iterator to
point to the new container.
This code works fine, except when `__cn1` and `__cn2` are the same container.
`__cn1->__add()` might need to grow the array of iterators, and when it does,
`__ip` becomes invalid, so the second line becomes a use-after-free error.
Simply swapping the order of the above two lines is not sufficient, because of
the memmove() below. The easiest and most performant solution is just to skip
touching any iterators if the containers are the same.
Differential Revision: https://reviews.llvm.org/D58926
Modified:
libcxx/trunk/include/list
libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
Modified: libcxx/trunk/include/list
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/list?rev=355550&r1=355549&r2=355550&view=diff
==============================================================================
--- libcxx/trunk/include/list (original)
+++ libcxx/trunk/include/list Wed Mar 6 13:10:08 2019
@@ -2013,22 +2013,24 @@ list<_Tp, _Alloc>::splice(const_iterator
base::__sz() += __c.__sz();
__c.__sz() = 0;
#if _LIBCPP_DEBUG_LEVEL >= 2
- __libcpp_db* __db = __get_db();
- __c_node* __cn1 = __db->__find_c_and_lock(this);
- __c_node* __cn2 = __db->__find_c(&__c);
- for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
- {
- --__ip;
- iterator* __i = static_cast<iterator*>((*__ip)->__i_);
- if (__i->__ptr_ != __c.__end_as_link())
+ if (&__c != this) {
+ __libcpp_db* __db = __get_db();
+ __c_node* __cn1 = __db->__find_c_and_lock(this);
+ __c_node* __cn2 = __db->__find_c(&__c);
+ for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
{
- __cn1->__add(*__ip);
- (*__ip)->__c_ = __cn1;
- if (--__cn2->end_ != __ip)
- memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ --__ip;
+ iterator* __i = static_cast<iterator*>((*__ip)->__i_);
+ if (__i->__ptr_ != __c.__end_as_link())
+ {
+ __cn1->__add(*__ip);
+ (*__ip)->__c_ = __cn1;
+ if (--__cn2->end_ != __ip)
+ memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ }
}
+ __db->unlock();
}
- __db->unlock();
#endif
}
}
@@ -2056,22 +2058,24 @@ list<_Tp, _Alloc>::splice(const_iterator
--__c.__sz();
++base::__sz();
#if _LIBCPP_DEBUG_LEVEL >= 2
- __libcpp_db* __db = __get_db();
- __c_node* __cn1 = __db->__find_c_and_lock(this);
- __c_node* __cn2 = __db->__find_c(&__c);
- for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
- {
- --__ip;
- iterator* __j = static_cast<iterator*>((*__ip)->__i_);
- if (__j->__ptr_ == __f)
+ if (&__c != this) {
+ __libcpp_db* __db = __get_db();
+ __c_node* __cn1 = __db->__find_c_and_lock(this);
+ __c_node* __cn2 = __db->__find_c(&__c);
+ for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
{
- __cn1->__add(*__ip);
- (*__ip)->__c_ = __cn1;
- if (--__cn2->end_ != __ip)
- memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ --__ip;
+ iterator* __j = static_cast<iterator*>((*__ip)->__i_);
+ if (__j->__ptr_ == __f)
+ {
+ __cn1->__add(*__ip);
+ (*__ip)->__c_ = __cn1;
+ if (--__cn2->end_ != __ip)
+ memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ }
}
+ __db->unlock();
}
- __db->unlock();
#endif
}
}
@@ -2110,26 +2114,28 @@ list<_Tp, _Alloc>::splice(const_iterator
base::__unlink_nodes(__first, __last);
__link_nodes(__p.__ptr_, __first, __last);
#if _LIBCPP_DEBUG_LEVEL >= 2
- __libcpp_db* __db = __get_db();
- __c_node* __cn1 = __db->__find_c_and_lock(this);
- __c_node* __cn2 = __db->__find_c(&__c);
- for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
- {
- --__ip;
- iterator* __j = static_cast<iterator*>((*__ip)->__i_);
- for (__link_pointer __k = __f.__ptr_;
- __k != __l.__ptr_; __k = __k->__next_)
+ if (&__c != this) {
+ __libcpp_db* __db = __get_db();
+ __c_node* __cn1 = __db->__find_c_and_lock(this);
+ __c_node* __cn2 = __db->__find_c(&__c);
+ for (__i_node** __ip = __cn2->end_; __ip != __cn2->beg_;)
{
- if (__j->__ptr_ == __k)
+ --__ip;
+ iterator* __j = static_cast<iterator*>((*__ip)->__i_);
+ for (__link_pointer __k = __f.__ptr_;
+ __k != __l.__ptr_; __k = __k->__next_)
{
- __cn1->__add(*__ip);
- (*__ip)->__c_ = __cn1;
- if (--__cn2->end_ != __ip)
- memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ if (__j->__ptr_ == __k)
+ {
+ __cn1->__add(*__ip);
+ (*__ip)->__c_ = __cn1;
+ if (--__cn2->end_ != __ip)
+ memmove(__ip, __ip+1, (__cn2->end_ - __ip)*sizeof(__i_node*));
+ }
}
}
+ __db->unlock();
}
- __db->unlock();
#endif
}
}
Modified: libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp?rev=355550&r1=355549&r2=355550&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp (original)
+++ libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp Wed Mar 6 13:10:08 2019
@@ -65,6 +65,7 @@ public:
}
if constexpr (CT == CT_List) {
SpliceFirstElem();
+ SpliceSameContainer();
}
} catch (...) {
assert(false && "uncaught debug exception");
@@ -110,6 +111,11 @@ private:
}
}
+ static void SpliceSameContainer() {
+ CHECKPOINT("splice(<same-container>)");
+ Container C = {1, 1};
+ C.splice(C.end(), C, C.begin());
+ }
static void SpliceFirstElemAfter() {
// See llvm.org/PR35564
More information about the libcxx-commits
mailing list