From e856aff526dd196b305d0a1a830aa971c34c5b01 Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Tue, 1 Oct 2019 15:46:29 +0200
Subject: [PATCH] COMMON: Fix btree node free race condition

The bt_node_free function frees the node by marking it
as free by setting flag BT_FLAG_FREE and adding it to
the free list. It also calls the specified callback
function with the node value.

To avoid a race condition, it must first set the flag to
mark the node deleted (i.e. free), and then call the callback
function. The callback function might free the node value.
If the node is not yet marked as deleted when the value is
being freed, then another thread might access the node and its
value, although the value has already been freed.

This problem may for example happen when one thread runs
C_FindObjectsInit, and another thread deletes an object via
C_DestroyObject at the same time.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 usr/lib/common/btree.c      | 14 ++++++++++----
 usr/lib/common/lock_btree.c | 14 ++++++++++----
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/usr/lib/common/btree.c b/usr/lib/common/btree.c
index e8612146..fba15ce4 100644
--- a/usr/lib/common/btree.c
+++ b/usr/lib/common/btree.c
@@ -197,10 +197,10 @@ struct btnode *bt_node_free(struct btree *t, unsigned long node_num,
                             void (*delete_func) (void *))
 {
     struct btnode *node = bt_get_node(t, node_num);
+    void  *value;
 
     if (node) {
-        if (delete_func)
-            (*delete_func) (node->value);
+        value = node->value;
 
         __transaction_atomic {  /* start transaction */
             node->flags |= BT_FLAG_FREE;
@@ -213,6 +213,9 @@ struct btnode *bt_node_free(struct btree *t, unsigned long node_num,
             t->free_list = node;
             t->free_nodes++;
         }                       /* end transaction */
+
+        if (delete_func)
+            (*delete_func) (value);
     }
 
     return node;
@@ -224,10 +227,10 @@ struct btnode *bt_node_free_(STDLL_TokData_t *tokdata, struct btree *t,
                                                   void *))
 {
     struct btnode *node = bt_get_node(t, node_num);
+    void *value;
 
     if (node) {
-        if (delete_func)
-            (*delete_func) (tokdata, node->value);
+        value = node->value;
 
         __transaction_atomic {  /* start transaction */
             node->flags |= BT_FLAG_FREE;
@@ -240,6 +243,9 @@ struct btnode *bt_node_free_(STDLL_TokData_t *tokdata, struct btree *t,
             t->free_list = node;
             t->free_nodes++;
         }                       /* end transaction */
+
+        if (delete_func)
+            (*delete_func) (tokdata, value);
     }
 
     return node;
diff --git a/usr/lib/common/lock_btree.c b/usr/lib/common/lock_btree.c
index e4f3c1a2..7884157c 100644
--- a/usr/lib/common/lock_btree.c
+++ b/usr/lib/common/lock_btree.c
@@ -224,6 +224,7 @@ struct btnode *bt_node_free(struct btree *t, unsigned long node_num,
 {
     struct btnode *node;
     int lock = 1;
+    void *value;
 
     if (pthread_rwlock_wrlock(&btree_rwlock))
         lock = 0;
@@ -231,8 +232,7 @@ struct btnode *bt_node_free(struct btree *t, unsigned long node_num,
     node = bt_get_node(t, node_num);
 
     if (node) {
-        if (delete_func)
-            (*delete_func) (node->value);
+        value = node->value;
 
         node->flags |= BT_FLAG_FREE;
 
@@ -243,6 +243,9 @@ struct btnode *bt_node_free(struct btree *t, unsigned long node_num,
         node->value = t->free_list;
         t->free_list = node;
         t->free_nodes++;
+
+        if (delete_func)
+            (*delete_func) (value);
     }
 
     if (lock)
@@ -257,6 +260,7 @@ struct btnode *bt_node_free_(STDLL_TokData_t *tokdata, struct btree *t,
                                                   void *))
 {
     struct btnode *node;
+    void *value;
 
     if (pthread_rwlock_wrlock(&btree_rwlock)) {
         TRACE_ERROR("Write Lock failed.\n");
@@ -266,8 +270,7 @@ struct btnode *bt_node_free_(STDLL_TokData_t *tokdata, struct btree *t,
     node = bt_get_node(t, node_num);
 
     if (node) {
-        if (delete_func)
-            (*delete_func) (tokdata, node->value);
+        value = node->value;
 
         node->flags |= BT_FLAG_FREE;
 
@@ -278,6 +281,9 @@ struct btnode *bt_node_free_(STDLL_TokData_t *tokdata, struct btree *t,
         node->value = t->free_list;
         t->free_list = node;
         t->free_nodes++;
+
+        if (delete_func)
+            (*delete_func) (tokdata, value);
     }
 
     pthread_rwlock_unlock(&btree_rwlock);
-- 
2.13.7

