summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Adam <obnox@samba.org>2012-04-12 13:38:32 +0200
committerAndreas Schneider <asn@samba.org>2012-04-25 14:31:10 +0200
commit8ddc2aa0e1734f00f120c4ac7a0e2ebbaa888f11 (patch)
tree084fffe314183c2dd1bd3fd975049cbc87fecade
parent705ea5b8c29a7ea9d1e2865abeec3d31d4c18d93 (diff)
downloadsamba-8ddc2aa0e1734f00f120c4ac7a0e2ebbaa888f11.tar.gz
samba-8ddc2aa0e1734f00f120c4ac7a0e2ebbaa888f11.tar.bz2
samba-8ddc2aa0e1734f00f120c4ac7a0e2ebbaa888f11.zip
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 Signed-off-by: Andreas Schneider <asn@samba.org>
-rw-r--r--source3/registry/reg_api.c39
1 files changed, 33 insertions, 6 deletions
diff --git a/source3/registry/reg_api.c b/source3/registry/reg_api.c
index fa00de503d..155fc96a5f 100644
--- a/source3/registry/reg_api.c
+++ b/source3/registry/reg_api.c
@@ -723,10 +723,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);
@@ -734,8 +741,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,
@@ -743,15 +752,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)