dbwrap: Protect against invalid db_record->value

After dbwrap_record_storev()/delete(), dbwrap_record_get_value()
information is stale. Assert on the attempt to re-fetch data after it
became stale. This can't protect against someone copying the result
from dbwrap_record_get_value() somewhere else, but it's better than
nothing.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index cc58397eb6e35c18180aed11ddf3fe5f378ec6a9..33f0b91ea03011d91ee5722f5ccb57273d47993d 100644 (file)
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -79,6 +79,7 @@ TDB_DATA dbwrap_record_get_key(const struct db_record *rec)
 
 TDB_DATA dbwrap_record_get_value(const struct db_record *rec)
 {
+       SMB_ASSERT(rec->value_valid);
        return rec->value;
 }
 
@@ -87,6 +88,12 @@ NTSTATUS dbwrap_record_storev(struct db_record *rec,
 {
        NTSTATUS status;
 
+       /*
+        * Invalidate before rec->storev() is called, give
+        * rec->storev() the chance to re-validate rec->value.
+        */
+       rec->value_valid = false;
+
        status = rec->storev(rec, dbufs, num_dbufs, flags);
        if (!NT_STATUS_IS_OK(status)) {
                return status;
@@ -103,6 +110,12 @@ NTSTATUS dbwrap_record_delete(struct db_record *rec)
 {
        NTSTATUS status;
 
+       /*
+        * Invalidate before rec->delete_rec() is called, give
+        * rec->delete_rec() the chance to re-validate rec->value.
+        */
+       rec->value_valid = false;
+
        status = rec->delete_rec(rec);
        if (!NT_STATUS_IS_OK(status)) {
                return status;

diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index a493b261dcbf5905cfd243643548626a5d73ffda..8a1f03c7becefb207966456eadb3e1d032994b5e 100644 (file)
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -29,6 +29,7 @@ struct tevent_req;
 struct db_record {
        struct db_context *db;
        TDB_DATA key, value;
+       bool value_valid;
        NTSTATUS (*storev)(struct db_record *rec, const TDB_DATA *dbufs,
                           int num_dbufs, int flag);
        NTSTATUS (*delete_rec)(struct db_record *rec);

diff --git a/lib/dbwrap/dbwrap_rbt.c b/lib/dbwrap/dbwrap_rbt.c
index 145cfccf0823960cd8a294ab04ffef3a217f7a2e..d988ca776de2246eb85cb6f0b06889e4c0d634bc 100644 (file)
--- a/lib/dbwrap/dbwrap_rbt.c
+++ b/lib/dbwrap/dbwrap_rbt.c
@@ -375,6 +375,7 @@ static struct db_record *db_rbt_fetch_locked(struct db_context *db_ctx,
 
        rec_priv->node = res.node;
        result->value  = res.val;
+       result->value_valid = true;
 
        if (found) {
                result->key = res.key;
@@ -447,6 +448,7 @@ static int db_rbt_traverse_internal(struct db_context *db,
                rec.storev = db_rbt_storev;
                rec.delete_rec = db_rbt_delete;
                db_rbt_parse_node(rec_priv.node, &rec.key, &rec.value);
+               rec.value_valid = true;
 
                if (rw) {
                        ctx->traverse_nextp = &next;

diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index b7bba40705a7ca417e2c462c259b6e9484f06952..e855f1ed2d615d63d120d835eef64b12fc06e4ba 100644 (file)
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -108,6 +108,7 @@ static int db_tdb_fetchlock_parse(TDB_DATA key, TDB_DATA data,
        else {
                result->value.dptr = NULL;
        }
+       result->value_valid = true;
 
        return 0;
 }
@@ -215,6 +216,7 @@ static NTSTATUS db_tdb_do_locked(struct db_context *db, TDB_DATA key,
                .db = db, .key = key,
                .value = (struct TDB_DATA) { .dptr = buf,
                                             .dsize = talloc_get_size(buf) },
+               .value_valid = true,
                .storev = db_tdb_storev, .delete_rec = db_tdb_delete,
                .private_data = ctx
        };
@@ -344,6 +346,7 @@ static int db_tdb_traverse_func(TDB_CONTEXT *tdb, TDB_DATA kbuf, TDB_DATA dbuf,
 
        rec.key = kbuf;
        rec.value = dbuf;
+       rec.value_valid = true;
        rec.storev = db_tdb_storev;
        rec.delete_rec = db_tdb_delete;
        rec.private_data = ctx->db->private_data;
@@ -387,6 +390,7 @@ static int db_tdb_traverse_read_func(TDB_CONTEXT *tdb, TDB_DATA kbuf, TDB_DATA d
 
        rec.key = kbuf;
        rec.value = dbuf;
+       rec.value_valid = true;
        rec.storev = db_tdb_storev_deny;
        rec.delete_rec = db_tdb_delete_deny;
        rec.private_data = ctx->db->private_data;

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 0e108920f585acbd4e210e1a7ea3b5afccb0638d..434a356feef5062dd36fd951969e38f954c8115a 100644 (file)
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -554,6 +554,7 @@ static struct db_record *db_ctdb_fetch_locked_transaction(struct db_ctdb_ctx *ct
                DEBUG(0, ("talloc failed\n"));
                TALLOC_FREE(result);
        }
+       result->value_valid = true;
 
        SAFE_FREE(ctdb_data.dptr);
 
@@ -1246,6 +1247,7 @@ again:
                        TALLOC_FREE(result);
                }
        }
+       result->value_valid = true;
 
        SAFE_FREE(ctdb_data.dptr);
 

diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 41cefbd153385dfa2da8ca546f5adccc4a92d40a..f406be178a3046628a771491e07e7cb99532d24b 100644 (file)
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -248,6 +248,7 @@ static struct db_record *dbwrap_watched_fetch_locked(
        rec->key = dbwrap_record_get_key(subrec->subrec);
        rec->storev = dbwrap_watched_storev;
        rec->delete_rec = dbwrap_watched_delete;
+       rec->value_valid = true;
 
        subrec_value = dbwrap_record_get_value(subrec->subrec);
 
@@ -351,6 +352,7 @@ static void dbwrap_watched_do_locked_fn(
                state->subrec.wrec.watchers = watchers;
 
                rec.value = state->subrec.wrec.data;
+               rec.value_valid = true;
        }
 
        state->fn(&rec, state->subrec.wrec.data, state->private_data);
@@ -556,6 +558,7 @@ static int dbwrap_watched_traverse_fn(struct db_record *rec,
        }
 
        prec.value = wrec.data;
+       prec.value_valid = true;
 
        return state->fn(&prec, state->private_data);
 }