Fixed memory-use-after-free bug in EllenBinTree<HP>

diff --git a/cds/intrusive/details/ellen_bintree_base.h b/cds/intrusive/details/ellen_bintree_base.h
index 08b23ac20418960348f761108409124cee29d76c..29c40ed73104649a4d416b02abcabc465b472127 100644 (file)
--- 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<update_desc_type *>( (++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 61b71367ecd3d517e39ad62bd6720f7aa0c030ae..20bcbd68ca7e66224e85336ec69c70d406d9cd9c 100644 (file)
--- 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<internal_node *>(p);})
+                    []( tree_node * p ) -> internal_node* { return static_cast<internal_node *>(p);})
                 : res.guards.protect( search_result::Guard_Leaf, pParent->m_pLeft,
-                                                []( tree_node * p ) -> internal_node* { return static_cast<internal_node *>(p);});
-            if ( p && p->is_leaf() )
-                res.guards.assign( search_result::Guard_Leaf, node_traits::to_value_ptr( static_cast<leaf_node *>( p )));
+                    []( tree_node * p ) -> internal_node* { return static_cast<internal_node *>(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<leaf_node *>(p));} )
+                : res.guards.protect( search_result::Guard_temporary, pParent->m_pLeft,
+                    []( tree_node * p ) -> value_type* { return node_traits::to_value_ptr( static_cast<leaf_node *>(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<leaf_node *>( 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<internal_node *>(
-                    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<leaf_node *>(
-                    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<internal_node *>(res.pGrandParent->get_child( res.bRightParent, memory_model::memory_order_relaxed )) == res.pParent
+                && static_cast<leaf_node *>( 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);