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>

4 files changed, 20 insertions, 0 deletions

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index cc58397eb6e..33f0b91ea03 100644
--- 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 a493b261dcb..8a1f03c7bec 100644
--- 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 145cfccf082..d988ca776de 100644
--- 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 b7bba40705a..e855f1ed2d6 100644
--- 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;