From 44e99f9dc7cc5001fad720c4b68ab58068574e56 Mon Sep 17 00:00:00 2001 From: Michael Adam 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 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 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 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 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; inum_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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 Autobuild-User: Stefan Metzmacher 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 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 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 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 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 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 (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 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 (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 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 (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 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 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