From 873b6f0f21e61be6e4f7084fc0ded768dfa077ab Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 19 Dec 2007 13:48:49 +0100 Subject: 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) --- source3/lib/dbwrap_rbt.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'source3') diff --git a/source3/lib/dbwrap_rbt.c b/source3/lib/dbwrap_rbt.c index 468b9405ee..93d73f29d1 100644 --- 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; -- cgit