Attachment 7535 Details for Bug 8921 – patchset against v3-6-test

[patch] patchset against v3-6-test

v3-6-patchset (text/plain), 43.40 KB, created by Gregor Beck (550 Unknown user) on 2012-05-09 09:36:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Gregor Beck (550 Unknown user)

Created: 2012-05-09 09:36:31 UTC

Size: 43.40 KB

patch

obsolete

>From 44e99f9dc7cc5001fad720c4b68ab58068574e56 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Sat, 5 May 2012 02:12:25 +0200
>Subject: [PATCH 01/29] s3:registry: implement values_need_update and
> subkeys_need_update in the smbconf backend
>
>It simply calls to the regdb functions.
>This fixes a caching issue uncovered by recent changes.
>(cherry picked from commit bff7589818e602ace6cd0a4125d5f6a2ba97cded)
>---
> source3/registry/reg_backend_smbconf.c |   11 +++++++++++
> 1 file changed, 11 insertions(+)
>
>diff --git a/source3/registry/reg_backend_smbconf.c b/source3/registry/reg_backend_smbconf.c
>index 912f5eb..44ffd42 100644
>--- a/source3/registry/reg_backend_smbconf.c
>+++ b/source3/registry/reg_backend_smbconf.c
>@@ -81,6 +81,15 @@ static WERROR smbconf_set_secdesc(const char *key,
> 	return regdb_ops.set_secdesc(key, secdesc);
> }
> 
>+static bool smbconf_subkeys_need_update(struct regsubkey_ctr *subkeys)
>+{
>+	return regdb_ops.subkeys_need_update(subkeys);
>+}
>+
>+static bool smbconf_values_need_update(struct regval_ctr *values)
>+{
>+	return regdb_ops.values_need_update(values);
>+}
> 
> /*
>  * Table of function pointers for accessing smb.conf data
>@@ -96,4 +105,6 @@ struct registry_ops smbconf_reg_ops = {
> 	.reg_access_check = smbconf_reg_access_check,
> 	.get_secdesc = smbconf_get_secdesc,
> 	.set_secdesc = smbconf_set_secdesc,
>+	.subkeys_need_update = smbconf_subkeys_need_update,
>+	.values_need_update = smbconf_values_need_update,
> };
>-- 
>1.7.10
>
>
>From 24b487cb7e47e143caef073c5925cc2537184ecb Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 00:10:14 +0200
>Subject: [PATCH 02/29] s3:registry:reg_api: fix reg_queryvalue to not fail
> when values are modified while it runs (cherry picked
> from commit 5d26120b5ab180212d570dd256e8989e0c80224d)
>
>---
> source3/registry/reg_api.c |   46 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 935d244..6bdd18b 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -375,6 +375,43 @@ WERROR reg_enumvalue(TALLOC_CTX *mem_ctx, struct registry_key *key,
> 	return WERR_OK;
> }
> 
>+static WERROR reg_enumvalue_nocachefill(TALLOC_CTX *mem_ctx,
>+					struct registry_key *key,
>+					uint32 idx, char **pname,
>+					struct registry_value **pval)
>+{
>+	struct registry_value *val;
>+	struct regval_blob *blob;
>+
>+	if (!(key->key->access_granted & KEY_QUERY_VALUE)) {
>+		return WERR_ACCESS_DENIED;
>+	}
>+
>+	if (idx >= regval_ctr_numvals(key->values)) {
>+		return WERR_NO_MORE_ITEMS;
>+	}
>+
>+	blob = regval_ctr_specific_value(key->values, idx);
>+
>+	val = talloc_zero(mem_ctx, struct registry_value);
>+	if (val == NULL) {
>+		return WERR_NOMEM;
>+	}
>+
>+	val->type = regval_type(blob);
>+	val->data = data_blob_talloc(mem_ctx, regval_data_p(blob), regval_size(blob));
>+
>+	if (pname
>+	    && !(*pname = talloc_strdup(
>+			 mem_ctx, regval_name(blob)))) {
>+		TALLOC_FREE(val);
>+		return WERR_NOMEM;
>+	}
>+
>+	*pval = val;
>+	return WERR_OK;
>+}
>+
> WERROR reg_queryvalue(TALLOC_CTX *mem_ctx, struct registry_key *key,
> 		      const char *name, struct registry_value **pval)
> {
>@@ -393,7 +430,14 @@ WERROR reg_queryvalue(TALLOC_CTX *mem_ctx, struct registry_key *key,
> 		struct regval_blob *blob;
> 		blob = regval_ctr_specific_value(key->values, i);
> 		if (strequal(regval_name(blob), name)) {
>-			return reg_enumvalue(mem_ctx, key, i, NULL, pval);
>+			/*
>+			 * don't use reg_enumvalue here:
>+			 * re-reading the values from the disk
>+			 * would change the indexing and break
>+			 * this function.
>+			 */
>+			return reg_enumvalue_nocachefill(mem_ctx, key, i,
>+							 NULL, pval);
> 		}
> 	}
> 
>-- 
>1.7.10
>
>
>From 98deeb2e567b49f164b66229d6dfbd21f1de3b74 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 01:00:51 +0200
>Subject: [PATCH 03/29] s4:torture:rpc:spoolss: also initialize driverName
> before checking it in test_PrinterData_DsSpooler()
> (cherry picked from commit
> 46428f96a4089925355b4eeebebb8d7f27e2ec0b)
>
>---
> source4/torture/rpc/spoolss.c |    1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
>index df800dc..75adaf4 100644
>--- a/source4/torture/rpc/spoolss.c
>+++ b/source4/torture/rpc/spoolss.c
>@@ -5499,6 +5499,7 @@ do {\
> 
> 	TEST_SET_SZ("description", comment, "newval");
> 	TEST_SET_SZ("location", location, "newval");
>+	TEST_SET_SZ("driverName", drivername, "newval");
> /*	TEST_SET_DWORD("priority", priority, 25); */
> 
> 	torture_assert(tctx,
>-- 
>1.7.10
>
>
>From e5cd9231a66300eac9b3cced801cad9d442b5456 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 14:33:39 +0200
>Subject: [PATCH 04/29] s3:registry: rename regval_ctr_key_exists() to
> regval_ctr_value_exists() (cherry picked from commit
> 60cdf3c8b5bbda9434f0d8a05fc581ab41b42d5c)
>
>---
> source3/registry/reg_backend_db.c |    4 ++--
> source3/registry/reg_objects.c    |    2 +-
> source3/registry/reg_objects.h    |    2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index 11bbe7e..f65f26d 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -320,7 +320,7 @@ static NTSTATUS init_registry_data_action(struct db_context *db,
> 
> 		/* preserve existing values across restarts. Only add new ones */
> 
>-		if (!regval_ctr_key_exists(values,
>+		if (!regval_ctr_value_exists(values,
> 					builtin_registry_values[i].valuename))
> 		{
> 			regdb_ctr_add_value(values,
>@@ -364,7 +364,7 @@ WERROR init_registry_data(void)
> 		regdb_fetch_values_internal(regdb,
> 					    builtin_registry_values[i].path,
> 					    values);
>-		if (!regval_ctr_key_exists(values,
>+		if (!regval_ctr_value_exists(values,
> 					builtin_registry_values[i].valuename))
> 		{
> 			TALLOC_FREE(values);
>diff --git a/source3/registry/reg_objects.c b/source3/registry/reg_objects.c
>index 980986f..de0de2c 100644
>--- a/source3/registry/reg_objects.c
>+++ b/source3/registry/reg_objects.c
>@@ -446,7 +446,7 @@ struct regval_blob *regval_ctr_specific_value(struct regval_ctr *ctr,
>  Check for the existance of a value
>  **********************************************************************/
> 
>-bool regval_ctr_key_exists(struct regval_ctr *ctr, const char *value)
>+bool regval_ctr_value_exists(struct regval_ctr *ctr, const char *value)
> {
> 	int 	i;
> 
>diff --git a/source3/registry/reg_objects.h b/source3/registry/reg_objects.h
>index a84bc8a..0599bfe 100644
>--- a/source3/registry/reg_objects.h
>+++ b/source3/registry/reg_objects.h
>@@ -55,7 +55,7 @@ char* regval_name(struct regval_blob *val);
> uint32_t regval_type(struct regval_blob *val);
> struct regval_blob* regval_ctr_specific_value(struct regval_ctr *ctr,
> 					      uint32_t idx);
>-bool regval_ctr_key_exists(struct regval_ctr *ctr, const char *value);
>+bool regval_ctr_value_exists(struct regval_ctr *ctr, const char *value);
> struct regval_blob *regval_compose(TALLOC_CTX *ctx, const char *name,
> 				   uint32_t type,
> 				   const uint8_t *data_p, size_t size);
>-- 
>1.7.10
>
>
>From 6f93e22c081a444b0399aea2a99cfdcb1627339b Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 14:39:50 +0200
>Subject: [PATCH 05/29] s3:registry: add a new function
> regval_ctr_value_byname()
>
>This is like regval_ctr_key_exists() but does not return bool,
>but the regval_blob instead, if found, and NULL if not found.
>(cherry picked from commit b037d5461a7a9a2e51a3dd2794fcc47dfcff4468)
>---
> source3/registry/reg_objects.c |   18 ++++++++++++++++++
> source3/registry/reg_objects.h |    2 ++
> 2 files changed, 20 insertions(+)
>
>diff --git a/source3/registry/reg_objects.c b/source3/registry/reg_objects.c
>index de0de2c..693ea74 100644
>--- a/source3/registry/reg_objects.c
>+++ b/source3/registry/reg_objects.c
>@@ -458,6 +458,24 @@ bool regval_ctr_value_exists(struct regval_ctr *ctr, const char *value)
> 	return False;
> }
> 
>+/**
>+ * Get a value by its name
>+ */
>+struct regval_blob *regval_ctr_value_byname(struct regval_ctr *ctr,
>+					    const char *value)
>+{
>+	int i;
>+
>+	for (i=0; i<ctr->num_values; i++) {
>+		if (strequal(ctr->values[i]->valuename,value)) {
>+			return ctr->values[i];
>+		}
>+	}
>+
>+	return NULL;
>+}
>+
>+
> /***********************************************************************
>  * compose a struct regval_blob from input data
>  **********************************************************************/
>diff --git a/source3/registry/reg_objects.h b/source3/registry/reg_objects.h
>index 0599bfe..406c8b8 100644
>--- a/source3/registry/reg_objects.h
>+++ b/source3/registry/reg_objects.h
>@@ -55,6 +55,8 @@ char* regval_name(struct regval_blob *val);
> uint32_t regval_type(struct regval_blob *val);
> struct regval_blob* regval_ctr_specific_value(struct regval_ctr *ctr,
> 					      uint32_t idx);
>+struct regval_blob *regval_ctr_value_byname(struct regval_ctr *ctr,
>+					    const char *value);
> bool regval_ctr_value_exists(struct regval_ctr *ctr, const char *value);
> struct regval_blob *regval_compose(TALLOC_CTX *ctx, const char *name,
> 				   uint32_t type,
>-- 
>1.7.10
>
>
>From 9e78a5e84b01c9b7e049ab182f19236585931993 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 15:14:01 +0200
>Subject: [PATCH 06/29] s3:registry: fix a debug message typo (cherry picked
> from commit 9f82e1175f28bdc1c09e7bd795699b29049a77e3)
>
>---
> source3/registry/reg_backend_db.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index f65f26d..10fc9ce 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1761,7 +1761,7 @@ static int regdb_fetch_values_internal(struct db_context *db, const char* key,
> 	TDB_DATA value;
> 	WERROR werr;
> 
>-	DEBUG(10,("regdb_fetch_values: Looking for value of key [%s] \n", key));
>+	DEBUG(10,("regdb_fetch_values: Looking for values of key [%s]\n", key));
> 
> 	if (!regdb_key_exists(db, key)) {
> 		goto done;
>-- 
>1.7.10
>
>
>From 6afa1d38b86a91eed500636bd10e8b4f4106f442 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 15:35:14 +0200
>Subject: [PATCH 07/29] s3:registry: improve log message in
> regdb_unpack_values() (cherry picked from commit
> ae441d97cdbe8e35cd342ba979bacc3757c06cb7)
>
>---
> source3/registry/reg_backend_db.c |    3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index 10fc9ce..c0c5bce 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1707,7 +1707,8 @@ static int regdb_unpack_values(struct regval_ctr *values, uint8 *buf, int buflen
> 				(uint8_t *)data_p, size);
> 		SAFE_FREE(data_p); /* 'B' option to tdb_unpack does a malloc() */
> 
>-		DEBUG(8,("specific: [%s], len: %d\n", valuename, size));
>+		DEBUG(10, ("regdb_unpack_values: value[%d]: name[%s] len[%d]\n",
>+			   i, valuename, size));
> 	}
> 
> 	return len;
>-- 
>1.7.10
>
>
>From 8757cf4809c608583dc8041635ae94c2ef586e28 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 30 Mar 2012 15:39:58 +0200
>Subject: [PATCH 08/29] s3:registry: fix debug message in
> regdb_store_values_internal() (cherry picked from
> commit c46403f74116708f2f8b1d531f5881bb9d7f2a84)
>
>---
> source3/registry/reg_backend_db.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index c0c5bce..2fa7f58 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1806,7 +1806,7 @@ static bool regdb_store_values_internal(struct db_context *db, const char *key,
> 	NTSTATUS status;
> 	bool result = false;
> 
>-	DEBUG(10,("regdb_store_values: Looking for value of key [%s] \n", key));
>+	DEBUG(10,("regdb_store_values: Looking for values of key [%s]\n", key));
> 
> 	if (!regdb_key_exists(db, key)) {
> 		goto done;
>-- 
>1.7.10
>
>
>From 6d8867eee130467e4e8b4132c06aaf1b75f220e8 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 08:18:04 +0200
>Subject: [PATCH 09/29] s3:registry: don't leak the old contents when updating
> the value cache (cherry picked from commit
> 0bf44361caace3a4974dafa305033fb926d0f6d6)
>
>---
> source3/registry/reg_api.c |    1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 6bdd18b..c6e6d8e 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -89,6 +89,7 @@ static WERROR fill_value_cache(struct registry_key *key)
> 		}
> 	}
> 
>+	TALLOC_FREE(key->values);
> 	werr = regval_ctr_init(key, &(key->values));
> 	W_ERROR_NOT_OK_RETURN(werr);
> 
>-- 
>1.7.10
>
>
>From 08a4f11f3a65ffe8ba1f09cc8dbb90463622d4e5 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 20 Apr 2012 15:19:47 +0200
>Subject: [PATCH 10/29] s3:registry: untangle assignment from check and add a
> debugmessage in reg_setvalue() (cherry picked from
> commit a81d399456eb86ffb60bed8704cd8c7864b742db)
>
>---
> source3/registry/reg_api.c |    4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index c6e6d8e..bdf5ae1 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -711,7 +711,9 @@ WERROR reg_setvalue(struct registry_key *key, const char *name,
> 		return WERR_ACCESS_DENIED;
> 	}
> 
>-	if (!W_ERROR_IS_OK(err = fill_value_cache(key))) {
>+	err = fill_value_cache(key);
>+	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_setvalue: Error filling value cache: %s\n", win_errstr(err)));
> 		return err;
> 	}
> 
>-- 
>1.7.10
>
>
>From 0fe17477b386fb6fd0102a3eb12c5f3898275898 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 13:38:32 +0200
>Subject: [PATCH 11/29] s3:registry: fix race in reg_setvalue that could lead
> to data corruption
>
>(there was no lock around fetching the values and storing them)
>
>The layering is wrong in that it uses regdb transactions in reg_api
>(cherry picked from commit 9220377ceebf05e756fd108cbd30b503598e0fb8)
>---
> source3/registry/reg_api.c |   39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index bdf5ae1..bd3cabb 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -711,10 +711,17 @@ WERROR reg_setvalue(struct registry_key *key, const char *name,
> 		return WERR_ACCESS_DENIED;
> 	}
> 
>+	err = regdb_transaction_start();
>+	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_setvalue: Failed to start transaction: %s\n",
>+			  win_errstr(err)));
>+		return err;
>+	}
>+
> 	err = fill_value_cache(key);
> 	if (!W_ERROR_IS_OK(err)) {
> 		DEBUG(0, ("reg_setvalue: Error filling value cache: %s\n", win_errstr(err)));
>-		return err;
>+		goto done;
> 	}
> 
> 	existing = regval_ctr_getvalue(key->values, name);
>@@ -722,8 +729,10 @@ WERROR reg_setvalue(struct registry_key *key, const char *name,
> 	if ((existing != NULL) &&
> 	    (regval_size(existing) == val->data.length) &&
> 	    (memcmp(regval_data_p(existing), val->data.data,
>-		    val->data.length) == 0)) {
>-		return WERR_OK;
>+		    val->data.length) == 0))
>+	{
>+		err = WERR_OK;
>+		goto done;
> 	}
> 
> 	res = regval_ctr_addvalue(key->values, name, val->type,
>@@ -731,15 +740,33 @@ WERROR reg_setvalue(struct registry_key *key, const char *name,
> 
> 	if (res == 0) {
> 		TALLOC_FREE(key->values);
>-		return WERR_NOMEM;
>+		err = WERR_NOMEM;
>+		goto done;
> 	}
> 
> 	if (!store_reg_values(key->key, key->values)) {
> 		TALLOC_FREE(key->values);
>-		return WERR_REG_IO_FAILURE;
>+		DEBUG(0, ("reg_setvalue: store_reg_values failed\n"));
>+		err = WERR_REG_IO_FAILURE;
>+		goto done;
> 	}
> 
>-	return WERR_OK;
>+	err = WERR_OK;
>+
>+done:
>+	if (W_ERROR_IS_OK(err)) {
>+		err = regdb_transaction_commit();
>+		if (!W_ERROR_IS_OK(err)) {
>+			DEBUG(0, ("reg_setvalue: Error committing transaction: %s\n", win_errstr(err)));
>+		}
>+	} else {
>+		WERROR err1 = regdb_transaction_cancel();
>+		if (!W_ERROR_IS_OK(err1)) {
>+			DEBUG(0, ("reg_setvalue: Error cancelling transaction: %s\n", win_errstr(err1)));
>+		}
>+	}
>+
>+	return err;
> }
> 
> static WERROR reg_value_exists(struct registry_key *key, const char *name)
>-- 
>1.7.10
>
>
>From 8b38e546ed6a27b6b136814672367f9af3dc85f4 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 17:46:02 +0200
>Subject: [PATCH 12/29] s3:registry: untangle assignment from check in
> reg_deletevalue() (cherry picked from commit
> 585746338bda22ff8337d41c8cc50533c5facf56)
>
>---
> source3/registry/reg_api.c |    3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index bd3cabb..660a1a4 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -790,7 +790,8 @@ WERROR reg_deletevalue(struct registry_key *key, const char *name)
> 		return WERR_ACCESS_DENIED;
> 	}
> 
>-	if (!W_ERROR_IS_OK(err = fill_value_cache(key))) {
>+	err = fill_value_cache(key);
>+	if (!W_ERROR_IS_OK(err)) {
> 		return err;
> 	}
> 
>-- 
>1.7.10
>
>
>From c614a71ca944eeba6a09bf8c712ba721ebe748a7 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 17:52:43 +0200
>Subject: [PATCH 13/29] s3:registry: wrap reg_deletevalue() in a transaction
>
>This is at the wrong layer, but if fixes a race potentially causing
>data corruption by concurrent access.
>(cherry picked from commit c1208c4a9c10b03579dca3bcd304709e631d3c25)
>---
> source3/registry/reg_api.c |   34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 660a1a4..2eced26 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -790,24 +790,50 @@ WERROR reg_deletevalue(struct registry_key *key, const char *name)
> 		return WERR_ACCESS_DENIED;
> 	}
> 
>-	err = fill_value_cache(key);
>+	err = regdb_transaction_start();
> 	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_deletevalue: Failed to start transaction: %s\n",
>+			  win_errstr(err)));
> 		return err;
> 	}
> 
>+	err = fill_value_cache(key);
>+	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_deletevalue; Error filling value cache: %s\n",
>+			  win_errstr(err)));
>+		goto done;
>+	}
>+
> 	err = reg_value_exists(key, name);
> 	if (!W_ERROR_IS_OK(err)) {
>-		return err;
>+		goto done;
> 	}
> 
> 	regval_ctr_delvalue(key->values, name);
> 
> 	if (!store_reg_values(key->key, key->values)) {
> 		TALLOC_FREE(key->values);
>-		return WERR_REG_IO_FAILURE;
>+		err = WERR_REG_IO_FAILURE;
>+		DEBUG(0, ("reg_deletevalue: store_reg_values failed\n"));
>+		goto done;
> 	}
> 
>-	return WERR_OK;
>+	err = WERR_OK;
>+
>+done:
>+	if (W_ERROR_IS_OK(err)) {
>+		err = regdb_transaction_commit();
>+		if (!W_ERROR_IS_OK(err)) {
>+			DEBUG(0, ("reg_deletevalue: Error committing transaction: %s\n", win_errstr(err)));
>+		}
>+	} else {
>+		WERROR err1 = regdb_transaction_cancel();
>+		if (!W_ERROR_IS_OK(err1)) {
>+			DEBUG(0, ("reg_deletevalue: Error cancelling transaction: %s\n", win_errstr(err1)));
>+		}
>+	}
>+
>+	return err;
> }
> 
> WERROR reg_getkeysecurity(TALLOC_CTX *mem_ctx, struct registry_key *key,
>-- 
>1.7.10
>
>
>From 3de8e7694d345e224a83ee7c24a8511dc6ea3736 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 17:58:26 +0200
>Subject: [PATCH 14/29] s3:registry: untangle assignments from checks in
> reg_createkey() (cherry picked from commit
> 4ac9625fe42ded0717aafdf6eec4c1b2217c3c68)
>
>---
> source3/registry/reg_api.c |    8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 2eced26..b8d8f2c 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -568,9 +568,13 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	char *path, *end;
> 	WERROR err;
> 
>-	if (!(mem_ctx = talloc_new(ctx))) return WERR_NOMEM;
>+	mem_ctx = talloc_new(ctx);
>+	if (mem_ctx == NULL) {
>+		return WERR_NOMEM;
>+	}
> 
>-	if (!(path = talloc_strdup(mem_ctx, subkeypath))) {
>+	path = talloc_strdup(mem_ctx, subkeypath);
>+	if (path == NULL) {
> 		err = WERR_NOMEM;
> 		goto done;
> 	}
>-- 
>1.7.10
>
>
>From b361a86b8b6b93cc96523398595d029cb1fd5aa1 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 22:17:35 +0200
>Subject: [PATCH 15/29] s3:registry: wrap reg_createkey() in a transaction
>
>This is wrong layering (calling into regdb_transaction* in the reg_api code)
>but fixes a potential race. It makes the multi-step create procedure atomic.
>
>This should completely be done in the backend.
>(cherry picked from commit 65d9b116d0283b010e9e3c9ecf185ca42850838e)
>---
> source3/registry/reg_api.c |   36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index b8d8f2c..c7777a1 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -579,6 +579,13 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 		goto done;
> 	}
> 
>+	err = regdb_transaction_start();
>+	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_createkey: failed to start transaction: %s\n",
>+			  win_errstr(err)));
>+		goto done;
>+	}
>+
> 	while ((end = strchr(path, '\\')) != NULL) {
> 		struct registry_key *tmp;
> 		enum winreg_CreateAction action;
>@@ -588,7 +595,7 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 		err = reg_createkey(mem_ctx, key, path,
> 				    KEY_ENUMERATE_SUB_KEYS, &tmp, &action);
> 		if (!W_ERROR_IS_OK(err)) {
>-			goto done;
>+			goto trans_done;
> 		}
> 
> 		if (key != parent) {
>@@ -609,14 +616,14 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 		if (paction != NULL) {
> 			*paction = REG_OPENED_EXISTING_KEY;
> 		}
>-		goto done;
>+		goto trans_done;
> 	}
> 
> 	if (!W_ERROR_EQUAL(err, WERR_BADFILE)) {
> 		/*
> 		 * Something but "notfound" has happened, so bail out
> 		 */
>-		goto done;
>+		goto trans_done;
> 	}
> 
> 	/*
>@@ -627,7 +634,7 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	err = reg_openkey(mem_ctx, key, "", KEY_CREATE_SUB_KEY,
> 			  &create_parent);
> 	if (!W_ERROR_IS_OK(err)) {
>-		goto done;
>+		goto trans_done;
> 	}
> 
> 	/*
>@@ -635,10 +642,14 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	 */
> 
> 	err = fill_subkey_cache(create_parent);
>-	if (!W_ERROR_IS_OK(err)) goto done;
>+	if (!W_ERROR_IS_OK(err)) {
>+		goto trans_done;
>+	}
> 
> 	err = create_reg_subkey(key->key, path);
>-	W_ERROR_NOT_OK_GOTO_DONE(err);
>+	if (!W_ERROR_IS_OK(err)) {
>+		goto trans_done;
>+	}
> 
> 	/*
> 	 * Now open the newly created key
>@@ -649,6 +660,19 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 		*paction = REG_CREATED_NEW_KEY;
> 	}
> 
>+trans_done:
>+	if (W_ERROR_IS_OK(err)) {
>+		err = regdb_transaction_commit();
>+		if (!W_ERROR_IS_OK(err)) {
>+			DEBUG(0, ("reg_createkey: Error committing transaction: %s\n", win_errstr(err)));
>+		}
>+	} else {
>+		WERROR err1 = regdb_transaction_cancel();
>+		if (!W_ERROR_IS_OK(err1)) {
>+			DEBUG(0, ("reg_createkey: Error cancelling transaction: %s\n", win_errstr(err1)));
>+		}
>+	}
>+
>  done:
> 	TALLOC_FREE(mem_ctx);
> 	return err;
>-- 
>1.7.10
>
>
>From 13c113767d69625a744c65339b933690bbf0c0a8 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 12 Apr 2012 22:53:24 +0200
>Subject: [PATCH 16/29] s3:registry: wrap reg_deletekey() into a transaction
>
>This is wrong layering but fixes a race condition.
>(cherry picked from commit e3ad0456515c97f6697190c86b8cec4af8e1e190)
>---
> source3/registry/reg_api.c |   28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index c7777a1..69b987a 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -695,12 +695,19 @@ WERROR reg_deletekey(struct registry_key *parent, const char *path)
> 	err = reg_openkey(mem_ctx, parent, name, REG_KEY_READ, &key);
> 	W_ERROR_NOT_OK_GOTO_DONE(err);
> 
>+	err = regdb_transaction_start();
>+	if (!W_ERROR_IS_OK(err)) {
>+		DEBUG(0, ("reg_deletekey: Error starting transaction: %s\n",
>+			  win_errstr(err)));
>+		goto done;
>+	}
>+
> 	err = fill_subkey_cache(key);
>-	W_ERROR_NOT_OK_GOTO_DONE(err);
>+	W_ERROR_NOT_OK_GOTO(err, trans_done);
> 
> 	if (regsubkey_ctr_numkeys(key->subkeys) > 0) {
> 		err = WERR_ACCESS_DENIED;
>-		goto done;
>+		goto trans_done;
> 	}
> 
> 	/* no subkeys - proceed with delete */
>@@ -710,7 +717,7 @@ WERROR reg_deletekey(struct registry_key *parent, const char *path)
> 
> 		err = reg_openkey(mem_ctx, parent, name,
> 				  KEY_CREATE_SUB_KEY, &tmp_key);
>-		W_ERROR_NOT_OK_GOTO_DONE(err);
>+		W_ERROR_NOT_OK_GOTO(err, trans_done);
> 
> 		parent = tmp_key;
> 		name = end+1;
>@@ -718,11 +725,24 @@ WERROR reg_deletekey(struct registry_key *parent, const char *path)
> 
> 	if (name[0] == '\0') {
> 		err = WERR_INVALID_PARAM;
>-		goto done;
>+		goto trans_done;
> 	}
> 
> 	err = delete_reg_subkey(parent->key, name);
> 
>+trans_done:
>+	if (W_ERROR_IS_OK(err)) {
>+		err = regdb_transaction_commit();
>+		if (!W_ERROR_IS_OK(err)) {
>+			DEBUG(0, ("reg_deletekey: Error committing transaction: %s\n", win_errstr(err)));
>+		}
>+	} else {
>+		WERROR err1 = regdb_transaction_cancel();
>+		if (!W_ERROR_IS_OK(err1)) {
>+			DEBUG(0, ("reg_deletekey: Error cancelling transaction: %s\n", win_errstr(err1)));
>+		}
>+	}
>+
> done:
> 	TALLOC_FREE(mem_ctx);
> 	return err;
>-- 
>1.7.10
>
>
>From 8a052a5bd5a15bc43f8692240815b358f1902877 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Wed, 11 Apr 2012 15:38:29 +0200
>Subject: [PATCH 17/29] s3:registry:db: update the value container seqnum
> after storing/deleting to prevent next read from
> going to disk if possible
>
>Note that this will currently only be effective in the local TDB implementation.
>For CTDB, this wont work since seqnum currently works differently there (needs
>fixing): For tdb, store and delete operations bump the db seqnum, while
>transaction commits don't. For ctdb, the seqnum is bumped by the transaction
>commit but not by store and delete operations.
>(cherry picked from commit 13347d11c0e918f82e7e3c21125acc5e241d389f)
>---
> source3/registry/reg_backend_db.c |   12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index 2fa7f58..aaa2241 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1805,6 +1805,7 @@ static bool regdb_store_values_internal(struct db_context *db, const char *key,
> 	int len;
> 	NTSTATUS status;
> 	bool result = false;
>+	WERROR werr;
> 
> 	DEBUG(10,("regdb_store_values: Looking for values of key [%s]\n", key));
> 
>@@ -1847,8 +1848,17 @@ static bool regdb_store_values_internal(struct db_context *db, const char *key,
> 	}
> 
> 	status = dbwrap_trans_store_bystring(db, keystr, data, TDB_REPLACE);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DEBUG(0, ("regdb_store_values_internal: error storing: %s\n", nt_errstr(status)));
>+		goto done;
>+	}
> 
>-	result = NT_STATUS_IS_OK(status);
>+	/*
>+	 * update the seqnum in the cache to prevent the next read
>+	 * from going to disk
>+	 */
>+	werr = regval_ctr_set_seqnum(values, db->get_seqnum(db));
>+	result = W_ERROR_IS_OK(status);
> 
> done:
> 	TALLOC_FREE(ctx);
>-- 
>1.7.10
>
>
>From 9ebf387d0d96e670c29efe8515b138ba79dd3a39 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Wed, 11 Apr 2012 16:02:44 +0200
>Subject: [PATCH 18/29] s3:registry: update the seqnum in the subkey cache at
> the end of regval_store_keys
>
>The purpose is to prevent next reads from going to disk.
>
>Note that this will currently only be effective with local tdbs, not
>with ctdb: For tdb, store and delete bump the seqnum while transaction
>commit does not. For ctdb, transaction commit bumps the seqnum, while
>store and delete don't... This needs fixing (in ctdb).
>(cherry picked from commit 16d83149c1b5620598edd37bbd1a73bebec82b6e)
>---
> source3/registry/reg_backend_db.c |    6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index aaa2241..e0fe7c1 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -984,7 +984,11 @@ static NTSTATUS regdb_store_keys_action(struct db_context *db,
> 		TALLOC_FREE(path);
> 	}
> 
>-	werr = WERR_OK;
>+	/*
>+	 * Update the seqnum in the container to possibly
>+	 * prevent next read from going to disk
>+	 */
>+	werr = regsubkey_ctr_set_seqnum(store_ctx->ctr, db->get_seqnum(db));
> 
> done:
> 	talloc_free(mem_ctx);
>-- 
>1.7.10
>
>
>From f5db025c98d99c6c7139d1305a3d3dd95eb716c4 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Wed, 11 Apr 2012 15:48:02 +0200
>Subject: [PATCH 19/29] s3:registry: fix seqnum race in fetch_values_internal
>
>This prevents race between fetching seqnum and key content.
>
>Because there is currently no way to atomically fetch the
>record along with the seqnum, I use a loop.
>This is far from optimal and should should ideally be done
>differently. But for now it fixes the race.
>(cherry picked from commit 13bccba3c2f6e6fdda2b4a40dd4b1e250a98a7ef)
>
>Conflicts:
>
>	source3/registry/reg_backend_db.c
>---
> source3/registry/reg_backend_db.c |   24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index e0fe7c1..c2c36dde 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1765,6 +1765,7 @@ static int regdb_fetch_values_internal(struct db_context *db, const char* key,
> 	int ret = 0;
> 	TDB_DATA value;
> 	WERROR werr;
>+	int seqnum[2], count;
> 
> 	DEBUG(10,("regdb_fetch_values: Looking for values of key [%s]\n", key));
> 
>@@ -1777,10 +1778,27 @@ static int regdb_fetch_values_internal(struct db_context *db, const char* key,
> 		goto done;
> 	}
> 
>-	werr = regval_ctr_set_seqnum(values, db->get_seqnum(db));
>-	W_ERROR_NOT_OK_GOTO_DONE(werr);
>+	ZERO_STRUCT(value);
>+	count = 0;
>+	seqnum[0] = db->get_seqnum(db);
>+
>+	do {
>+		count++;
>+		TALLOC_FREE(value.dptr);
>+		value = regdb_fetch_key_internal(db, ctx, keystr);
>+		seqnum[count % 2] = db->get_seqnum(db);
>+	} while (seqnum[0] != seqnum[1]);
>+
>+	if (count > 1) {
>+		DEBUG(5, ("regdb_fetch_values_internal: it took %d attempts "
>+			  "to fetch key '%s' with constant seqnum\n",
>+			  count, key));
>+	}
> 
>-	value = regdb_fetch_key_internal(db, ctx, keystr);
>+	werr = regval_ctr_set_seqnum(values, seqnum[0]);
>+	if (!W_ERROR_IS_OK(werr)) {
>+		goto done;
>+	}
> 
> 	if (!value.dptr) {
> 		/* all keys have zero values by default */
>-- 
>1.7.10
>
>
>From 3b7944cc8e816bf130219eba9692929cdfbeddf1 Mon Sep 17 00:00:00 2001
>From: Gregor Beck <gbeck@sernet.de>
>Date: Wed, 13 Jul 2011 16:51:54 +0200
>Subject: [PATCH 20/29] s3:registry avoid pruning the sequencenumber while
> flushing the regsubkey_ctr
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>
>Autobuild-User: Stefan Metzmacher <metze@samba.org>
>Autobuild-Date: Fri Jul 15 08:34:47 CEST 2011 on sn-devel-104
>(cherry picked from commit 5049e3e142977a4c3d0f5a0fd9c06429f4d85bed)
>---
> source3/registry/reg_backend_db.c |    6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index c2c36dde..995da66 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1624,6 +1624,9 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
> 		goto done;
> 	}
> 
>+	werr = regsubkey_ctr_reinit(ctr);
>+	W_ERROR_NOT_OK_GOTO_DONE(werr);
>+
> 	werr = regsubkey_ctr_set_seqnum(ctr, db->get_seqnum(db));
> 	W_ERROR_NOT_OK_GOTO_DONE(werr);
> 
>@@ -1643,9 +1646,6 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
> 		goto done;
> 	}
> 
>-	werr = regsubkey_ctr_reinit(ctr);
>-	W_ERROR_NOT_OK_GOTO_DONE(werr);
>-
> 	for (i=0; i<num_items; i++) {
> 		len += tdb_unpack(buf+len, buflen-len, "f", subkeyname);
> 		werr = regsubkey_ctr_addkey(ctr, subkeyname);
>-- 
>1.7.10
>
>
>From 608174fd57f1c20eed83608eb9d0602c8344fc47 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Wed, 11 Apr 2012 15:51:40 +0200
>Subject: [PATCH 21/29] s3:registry: fix seqnum race in
> regdb_fetch_keys_internal
>
>This prevents race between fetching seqnum and key content.
>
>Because there is currently no way to atomically fetch the
>record along with the seqnum, I use a loop.
>This is far from optimal and should should ideally be done
>differently. But for now it fixes the race.
>(cherry picked from commit 66fcac5e479a530091ecb43d9f8cf90f4351ad17)
>---
> source3/registry/reg_backend_db.c |   25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index 995da66..68cd094 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1615,6 +1615,7 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
> 	fstring subkeyname;
> 	TALLOC_CTX *frame = talloc_stackframe();
> 	TDB_DATA value;
>+	int seqnum[2], count;
> 
> 	DEBUG(11,("regdb_fetch_keys: Enter key => [%s]\n", key ? key : "NULL"));
> 
>@@ -1627,10 +1628,28 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
> 	werr = regsubkey_ctr_reinit(ctr);
> 	W_ERROR_NOT_OK_GOTO_DONE(werr);
> 
>-	werr = regsubkey_ctr_set_seqnum(ctr, db->get_seqnum(db));
>-	W_ERROR_NOT_OK_GOTO_DONE(werr);
>+	count = 0;
>+	ZERO_STRUCT(value);
>+	seqnum[0] = db->get_seqnum(db);
>+
>+	do {
>+		count++;
>+		TALLOC_FREE(value.dptr);
>+		value = regdb_fetch_key_internal(db, frame, key);
>+		seqnum[count % 2] = db->get_seqnum(db);
> 
>-	value = regdb_fetch_key_internal(db, frame, key);
>+	} while (seqnum[0] != seqnum[1]);
>+
>+	if (count > 1) {
>+		DEBUG(5, ("regdb_fetch_keys_internal: it took %d attempts to "
>+			  "fetch key '%s' with constant seqnum\n",
>+			  count, key));
>+	}
>+
>+	werr = regsubkey_ctr_set_seqnum(ctr, seqnum[0]);
>+	if (!W_ERROR_IS_OK(werr)) {
>+		goto done;
>+	}
> 
> 	if (value.dsize == 0 || value.dptr == NULL) {
> 		DEBUG(10, ("regdb_fetch_keys: no subkeys found for key [%s]\n",
>-- 
>1.7.10
>
>
>From 2241bbf1539fad2a14feb4188c2c5093057d5d1e Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 15:29:41 +0200
>Subject: [PATCH 22/29] s3:registry: untangle assignment from check in
> regkey_open_onelevel() (cherry picked from commit
> 52d3c5c14898b5f2514d1512289370eb6f6fd369)
>
>---
> source3/registry/reg_api.c |    3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 69b987a..c81d05d 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -151,7 +151,8 @@ static WERROR regkey_open_onelevel(TALLOC_CTX *mem_ctx,
> 		goto done;
> 	}
> 
>-	if ( !(W_ERROR_IS_OK(result = regdb_open())) ) {
>+	result = regdb_open();
>+	if (!(W_ERROR_IS_OK(result))) {
> 		goto done;
> 	}
> 
>-- 
>1.7.10
>
>
>From 18d08dd408cb929a71d70a5228f58b7a3377c435 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 15:30:38 +0200
>Subject: [PATCH 23/29] s3:registry untangle an assignment from the check in
> regkey_open_onelevel() (cherry picked from commit
> 12b7b4f0a7d8607dc206c32a3822d5678c14d43b)
>
>---
> source3/registry/reg_api.c |    3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index c81d05d..592f4e3 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -195,7 +195,8 @@ static WERROR regkey_open_onelevel(TALLOC_CTX *mem_ctx,
> 
> 	/* Look up the table of registry I/O operations */
> 
>-	if ( !(key->ops = reghook_cache_find( key->name )) ) {
>+	key->ops = reghook_cache_find( key->name );
>+	if (key->ops == NULL) {
> 		DEBUG(0,("reg_open_onelevel: Failed to assign "
> 			 "registry_ops to [%s]\n", key->name ));
> 		result = WERR_BADFILE;
>-- 
>1.7.10
>
>
>From 74766dace06ef983e00bb1088d1d37f7d855755b Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 15:47:33 +0200
>Subject: [PATCH 24/29] s3:registry: convert reg_openkey() to use talloc
> instead of SMB_STRDUP etc (cherry picked from commit
> 42dd99d85ca04c10691f78d6340c6b702ade974b)
>
>---
> source3/registry/reg_api.c |   27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 592f4e3..bfc7aec 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -259,13 +259,15 @@ WERROR reg_openkey(TALLOC_CTX *mem_ctx, struct registry_key *parent,
> {
> 	struct registry_key *direct_parent = parent;
> 	WERROR err;
>-	char *p, *path, *to_free;
>+	char *p, *path;
> 	size_t len;
>+	TALLOC_CTX *frame = talloc_stackframe();
> 
>-	if (!(path = SMB_STRDUP(name))) {
>-		return WERR_NOMEM;
>+	path = talloc_strdup(frame, name);
>+	if (path == NULL) {
>+		err = WERR_NOMEM;
>+		goto error;
> 	}
>-	to_free = path;
> 
> 	len = strlen(path);
> 
>@@ -277,22 +279,19 @@ WERROR reg_openkey(TALLOC_CTX *mem_ctx, struct registry_key *parent,
> 		char *name_component;
> 		struct registry_key *tmp;
> 
>-		if (!(name_component = SMB_STRNDUP(path, (p - path)))) {
>+		name_component = talloc_strndup(frame, path, (p - path));
>+		if (name_component == NULL) {
> 			err = WERR_NOMEM;
> 			goto error;
> 		}
> 
>-		err = regkey_open_onelevel(mem_ctx, direct_parent,
>+		err = regkey_open_onelevel(frame, direct_parent,
> 					   name_component, parent->token,
> 					   KEY_ENUMERATE_SUB_KEYS, &tmp);
>-		SAFE_FREE(name_component);
> 
> 		if (!W_ERROR_IS_OK(err)) {
> 			goto error;
> 		}
>-		if (direct_parent != parent) {
>-			TALLOC_FREE(direct_parent);
>-		}
> 
> 		direct_parent = tmp;
> 		path = p+1;
>@@ -300,11 +299,9 @@ WERROR reg_openkey(TALLOC_CTX *mem_ctx, struct registry_key *parent,
> 
> 	err = regkey_open_onelevel(mem_ctx, direct_parent, path, parent->token,
> 				   desired_access, pkey);
>- error:
>-	if (direct_parent != parent) {
>-		TALLOC_FREE(direct_parent);
>-	}
>-	SAFE_FREE(to_free);
>+
>+error:
>+	talloc_free(frame);
> 	return err;
> }
> 
>-- 
>1.7.10
>
>
>From 65b9b8638bdf22b05f563ebbd7f9c0a0931a502b Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 16:05:33 +0200
>Subject: [PATCH 25/29] s3:registry: let fill_subkey_cache return WERR_BADFILE
> when the subkey list could not be loaded
>
>WERR_NO_MORE_ITEMS seems inappropriate.
>
>Pair-Programmed-With: Gregor Beck <gbeck@sernet.de>
>(cherry picked from commit 4b3dca83bf0da405524a64ca19771fd747ebe267)
>---
> source3/registry/reg_api.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index bfc7aec..62cc45f 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -116,7 +116,7 @@ static WERROR fill_subkey_cache(struct registry_key *key)
> 
> 	if (fetch_reg_keys(key->key, key->subkeys) == -1) {
> 		TALLOC_FREE(key->subkeys);
>-		return WERR_NO_MORE_ITEMS;
>+		return WERR_BADFILE;
> 	}
> 
> 	return WERR_OK;
>-- 
>1.7.10
>
>
>From beb24b585e0ea703d318fc499a19433b038d0a74 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 16:07:21 +0200
>Subject: [PATCH 26/29] s3:registry: use fill_subkey_cache to check exsistence
> in regkey_open_onelevel().
>
>Pair-Programmed-With: Gregor Beck <gbeck@sernet.de>
>(cherry picked from commit af9d70fbce541c382a5fc54b1cc1af9b0b60a692)
>---
> source3/registry/reg_api.c |   13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 62cc45f..42fc4cb 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -137,7 +137,6 @@ static WERROR regkey_open_onelevel(TALLOC_CTX *mem_ctx,
> 	WERROR     	result = WERR_OK;
> 	struct registry_key *regkey;
> 	struct registry_key_handle *key;
>-	struct regsubkey_ctr	*subkeys = NULL;
> 
> 	DEBUG(7,("regkey_open_onelevel: name = [%s]\n", name));
> 
>@@ -203,21 +202,13 @@ static WERROR regkey_open_onelevel(TALLOC_CTX *mem_ctx,
> 		goto done;
> 	}
> 
>-	/* check if the path really exists; failed is indicated by -1 */
>-	/* if the subkey count failed, bail out */
>+	/* FIXME: Existence is currently checked by fetching the subkeys */
> 
>-	result = regsubkey_ctr_init(key, &subkeys);
>+	result = fill_subkey_cache(regkey);
> 	if (!W_ERROR_IS_OK(result)) {
> 		goto done;
> 	}
> 
>-	if ( fetch_reg_keys( key, subkeys ) == -1 )  {
>-		result = WERR_BADFILE;
>-		goto done;
>-	}
>-
>-	TALLOC_FREE( subkeys );
>-
> 	if ( !regkey_access_check( key, access_desired, &key->access_granted,
> 				   token ) ) {
> 		result = WERR_ACCESS_DENIED;
>-- 
>1.7.10
>
>
>From e6b0d636d5a8a8d4019f14676d01b10b0d5a0434 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 16:13:29 +0200
>Subject: [PATCH 27/29] s3:registry: remove a superfluous fill_subkey_cache()
> in reg_createkey()
>
>Pair-Programmed-With: Gregor Beck <gbeck@sernet.de>
>(cherry picked from commit 03ae7117df2ae42213a3ef9a5ea3adad2bf264e0)
>---
> source3/registry/reg_api.c |    5 -----
> 1 file changed, 5 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 42fc4cb..0363ca4 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -631,11 +631,6 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	 * Actually create the subkey
> 	 */
> 
>-	err = fill_subkey_cache(create_parent);
>-	if (!W_ERROR_IS_OK(err)) {
>-		goto trans_done;
>-	}
>-
> 	err = create_reg_subkey(key->key, path);
> 	if (!W_ERROR_IS_OK(err)) {
> 		goto trans_done;
>-- 
>1.7.10
>
>
>From aa460b5f930ccc284b5973b9719b50f0611949f5 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 23 Apr 2012 16:44:15 +0200
>Subject: [PATCH 28/29] s3:registry: replace call to reg_openkey() in
> reg_createkey() by accesscheck. (cherry picked from
> commit c1cc15c33be8926ffef173b514d0fb260292d9a3)
>
>---
> source3/registry/reg_api.c |   18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
>index 0363ca4..55e364f 100644
>--- a/source3/registry/reg_api.c
>+++ b/source3/registry/reg_api.c
>@@ -557,6 +557,7 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	TALLOC_CTX *mem_ctx;
> 	char *path, *end;
> 	WERROR err;
>+	uint32_t access_granted;
> 
> 	mem_ctx = talloc_new(ctx);
> 	if (mem_ctx == NULL) {
>@@ -617,14 +618,15 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	}
> 
> 	/*
>-	 * We have to make a copy of the current key, as we opened it only
>-	 * with ENUM_SUBKEY access.
>+	 * We may (e.g. in the iteration) have opened the key with ENUM_SUBKEY.
>+	 * Instead of re-opening the key with CREATE_SUB_KEY, we simply
>+	 * duplicate the access check here and skip the expensive full open.
> 	 */
>-
>-	err = reg_openkey(mem_ctx, key, "", KEY_CREATE_SUB_KEY,
>-			  &create_parent);
>-	if (!W_ERROR_IS_OK(err)) {
>-		goto trans_done;
>+	if (!regkey_access_check(key->key, KEY_CREATE_SUB_KEY, &access_granted,
>+				 key->token))
>+	{
>+		err = WERR_ACCESS_DENIED;
>+		goto done;
> 	}
> 
> 	/*
>@@ -640,7 +642,7 @@ WERROR reg_createkey(TALLOC_CTX *ctx, struct registry_key *parent,
> 	 * Now open the newly created key
> 	 */
> 
>-	err = reg_openkey(ctx, create_parent, path, desired_access, pkey);
>+	err = reg_openkey(ctx, key, path, desired_access, pkey);
> 	if (W_ERROR_IS_OK(err) && (paction != NULL)) {
> 		*paction = REG_CREATED_NEW_KEY;
> 	}
>-- 
>1.7.10
>
>
>From c55150b41a3f6b964a4d00a0509f0d5dcd79f6c8 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 4 May 2012 18:01:00 +0200
>Subject: [PATCH 29/29] s3:registry: return error when Key does not exist in
> regdb_fetch_values_internal() (cherry picked from
> commit 8a723ddfc1645e52830fb5f47a34f032f9c38931)
>
>---
> source3/registry/reg_backend_db.c |    3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
>index 68cd094..5855973 100644
>--- a/source3/registry/reg_backend_db.c
>+++ b/source3/registry/reg_backend_db.c
>@@ -1789,6 +1789,9 @@ static int regdb_fetch_values_internal(struct db_context *db, const char* key,
> 	DEBUG(10,("regdb_fetch_values: Looking for values of key [%s]\n", key));
> 
> 	if (!regdb_key_exists(db, key)) {
>+		DEBUG(10, ("regb_fetch_values: key [%s] does not exist\n",
>+			   key));
>+		ret = -1;
> 		goto done;
> 	}
> 
>-- 
>1.7.10
>

Flags:

asn: review+
obnox: review+

Actions: View

Attachments on bug 8921: 7535 | 7546 | 7547