Samba Shared Repository - branch v3-2-test updated - initial-v3-2-unstable-716-g12cce3b

On Tue, Dec 18, 2007 at 06:04:32PM -0600, Jeremy Allison wrote:
>     Fix valgrind error in dbwrap_rbt where rec_priv->node was
>     being accessed after free. VALOKER PLEASE CHECK THIS VERY
>     CAREFULLY !!!! This is a correct fix in that it fixes the
>     valgrind error, but it looks inelegant to me. I think if
>     I understood this code better I could craft a more subtle
>     fix. Still looking at it....

Thanks a lot. Fully correct. What about the attached little
simplification?

Volker
(This used to be commit 5b72828600fb057a7aeb5f1a6fb6c23c23f28cd8)

diff --git a/source3/lib/dbwrap_rbt.c b/source3/lib/dbwrap_rbt.c
index 468b9405eee15a2da745772b45e370bc55e90441..93d73f29d137065975e8044fe619b2703cf3209c 100644 (file)
--- a/source3/lib/dbwrap_rbt.c
+++ b/source3/lib/dbwrap_rbt.c
@@ -68,8 +68,6 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
 
        TDB_DATA this_key, this_val;
 
-       bool del_old_keyval = false;
-
        if (rec_priv->node != NULL) {
 
                /*
@@ -97,7 +95,11 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
                 */
 
                rb_erase(&rec_priv->node->rb_node, &rec_priv->db_ctx->tree);
-               del_old_keyval = true;
+
+               /*
+                * Keep the existing node around for a while: If the record
+                * existed before, we reference the key data in there.
+                */
        }
 
        node = (struct db_rbt_node *)SMB_MALLOC(
@@ -105,9 +107,7 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
                + data.dsize);
 
        if (node == NULL) {
-               if (del_old_keyval) {
-                       SAFE_FREE(rec_priv->node);
-               }
+               SAFE_FREE(rec_priv->node);
                return NT_STATUS_NO_MEMORY;
        }
 
@@ -119,11 +119,9 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
        db_rbt_parse_node(node, &this_key, &this_val);
 
        memcpy(this_key.dptr, rec->key.dptr, node->keysize);
-       memcpy(this_val.dptr, data.dptr, node->valuesize);
+       SAFE_FREE(rec_priv->node);
 
-       if (del_old_keyval) {
-               SAFE_FREE(rec_priv->node);
-       }
+       memcpy(this_val.dptr, data.dptr, node->valuesize);
 
        parent = NULL;
        p = &rec_priv->db_ctx->tree.rb_node;