From: khizmax Date: Mon, 16 Nov 2015 20:20:34 +0000 (+0300) Subject: Fixed memory-use-after-free bug in EllenBinTree X-Git-Tag: v2.1.0~67 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=36b0d16b11daa8cf5709ea275de5f55ccbbd40b5;p=libcds.git Fixed memory-use-after-free bug in EllenBinTree --- diff --git a/cds/intrusive/details/ellen_bintree_base.h b/cds/intrusive/details/ellen_bintree_base.h index 08b23ac2..29c40ed7 100644 --- a/cds/intrusive/details/ellen_bintree_base.h +++ b/cds/intrusive/details/ellen_bintree_base.h @@ -221,6 +221,11 @@ namespace cds { namespace intrusive { { return update_ptr( reinterpret_cast( (++m_nEmptyUpdate << 2) & 0xFFFF ) ); } + + base_class * get_child( bool bRight, atomics::memory_order mo ) const + { + return bRight ? m_pRight.load( mo ) : m_pLeft.load( mo ); + } //@endcond }; @@ -397,33 +402,33 @@ namespace cds { namespace intrusive { /// EllenBinTree empty statistics struct empty_stat { //@cond - void onInternalNodeCreated() {} - void onInternalNodeDeleted() {} - void onUpdateDescCreated() {} - void onUpdateDescDeleted() {} - void onInsertSuccess() {} - void onInsertFailed() {} - void onInsertRetry() {} - void onEnsureExist() {} - void onEnsureNew() {} - void onEnsureRetry() {} - void onEraseSuccess() {} - void onEraseFailed() {} - void onEraseRetry() {} - void onExtractMinSuccess() {} - void onExtractMinFailed() {} - void onExtractMinRetry() {} - void onExtractMaxSuccess() {} - void onExtractMaxFailed() {} - void onExtractMaxRetry() {} - void onFindSuccess() {} - void onFindFailed() {} - void onSearchRetry() {} - void onHelpInsert() {} - void onHelpDelete() {} - void onHelpMark() {} - void onHelpGuardSuccess() {} - void onHelpGuardFailed() {} + void onInternalNodeCreated() const {} + void onInternalNodeDeleted() const {} + void onUpdateDescCreated() const {} + void onUpdateDescDeleted() const {} + void onInsertSuccess() const {} + void onInsertFailed() const {} + void onInsertRetry() const {} + void onEnsureExist() const {} + void onEnsureNew() const {} + void onEnsureRetry() const {} + void onEraseSuccess() const {} + void onEraseFailed() const {} + void onEraseRetry() const {} + void onExtractMinSuccess() const {} + void onExtractMinFailed() const {} + void onExtractMinRetry() const {} + void onExtractMaxSuccess() const {} + void onExtractMaxFailed() const {} + void onExtractMaxRetry() const {} + void onFindSuccess() const {} + void onFindFailed() const {} + void onSearchRetry() const {} + void onHelpInsert() const {} + void onHelpDelete() const {} + void onHelpMark() const {} + void onHelpGuardSuccess() const {} + void onHelpGuardFailed() const {} //@endcond }; diff --git a/cds/intrusive/impl/ellen_bintree.h b/cds/intrusive/impl/ellen_bintree.h index 61b71367..20bcbd68 100644 --- a/cds/intrusive/impl/ellen_bintree.h +++ b/cds/intrusive/impl/ellen_bintree.h @@ -160,7 +160,7 @@ namespace cds { namespace intrusive { typedef typename traits::node_allocator node_allocator; ///< Allocator for internal node typedef typename traits::update_desc_allocator update_desc_allocator; ///< Update descriptor allocator - static CDS_CONSTEXPR const size_t c_nHazardPtrCount = 8; ///< Count of hazard pointer required for the algorithm + static CDS_CONSTEXPR const size_t c_nHazardPtrCount = 9; ///< Count of hazard pointer required for the algorithm protected: //@cond @@ -176,6 +176,7 @@ namespace cds { namespace intrusive { Guard_Leaf, Guard_updGrandParent, Guard_updParent, + Guard_temporary, // end of guard indices guard_count @@ -894,15 +895,23 @@ namespace cds { namespace intrusive { return false; } - static tree_node * protect_child_node( search_result& res, internal_node * pParent, bool bRight, update_ptr updParent ) + tree_node * protect_child_node( search_result& res, internal_node * pParent, bool bRight, update_ptr updParent ) const { + retry: tree_node * p = bRight ? res.guards.protect( search_result::Guard_Leaf, pParent->m_pRight, - []( tree_node * p ) -> internal_node* { return static_cast(p);}) + []( tree_node * p ) -> internal_node* { return static_cast(p);}) : res.guards.protect( search_result::Guard_Leaf, pParent->m_pLeft, - []( tree_node * p ) -> internal_node* { return static_cast(p);}); - if ( p && p->is_leaf() ) - res.guards.assign( search_result::Guard_Leaf, node_traits::to_value_ptr( static_cast( p ))); + []( tree_node * p ) -> internal_node* { return static_cast(p);}); + + // If we use member hook, data node pointer != internal node pointer + // So, we need protect the child twice: as internal node and as data node + // and then analyze what kind of node we have + tree_node * pVal = bRight + ? res.guards.protect( search_result::Guard_temporary, pParent->m_pRight, + []( tree_node * p ) -> value_type* { return node_traits::to_value_ptr( static_cast(p));} ) + : res.guards.protect( search_result::Guard_temporary, pParent->m_pLeft, + []( tree_node * p ) -> value_type* { return node_traits::to_value_ptr( static_cast(p));} ); // child node is guarded // See whether pParent->m_pUpdate has not been changed @@ -910,6 +919,15 @@ namespace cds { namespace intrusive { // update has been changed - returns nullptr as a flag to search retry return nullptr; } + + if ( p != pVal ) + goto retry; + + if ( p && p->is_leaf()) + res.guards.assign( search_result::Guard_Leaf, node_traits::to_value_ptr( static_cast( p ))); + + res.guards.clear( search_result::Guard_temporary ); + return p; } @@ -1132,18 +1150,8 @@ namespace cds { namespace intrusive { assert( res.pGrandParent != nullptr ); - return - static_cast( - res.bRightParent - ? res.pGrandParent->m_pRight.load(memory_model::memory_order_relaxed) - : res.pGrandParent->m_pLeft.load(memory_model::memory_order_relaxed) - ) == res.pParent - && - static_cast( - res.bRightLeaf - ? res.pParent->m_pRight.load(memory_model::memory_order_relaxed) - : res.pParent->m_pLeft.load(memory_model::memory_order_relaxed) - ) == res.pLeaf; + return static_cast(res.pGrandParent->get_child( res.bRightParent, memory_model::memory_order_relaxed )) == res.pParent + && static_cast( res.pParent->get_child( res.bRightLeaf, memory_model::memory_order_relaxed )) == res.pLeaf; } bool help_delete( update_desc * pOp ) @@ -1217,9 +1225,7 @@ namespace cds { namespace intrusive { assert( res.pLeaf->is_leaf() ); // check search result - if ( (res.bRightLeaf - ? res.pParent->m_pRight.load( memory_model::memory_order_acquire ) - : res.pParent->m_pLeft.load( memory_model::memory_order_acquire )) == res.pLeaf ) { + if ( res.pParent->get_child( res.bRightLeaf, memory_model::memory_order_acquire ) == res.pLeaf ) { leaf_node * pNewLeaf = node_traits::to_node_ptr( val ); int nCmp = node_compare()(val, *res.pLeaf);