diff options
| author | Michael Adam <obnox@samba.org> | 2012-04-12 13:38:32 +0200 |
|---|---|---|
| committer | Andreas Schneider <asn@samba.org> | 2012-04-25 14:31:10 +0200 |
| commit | 8ddc2aa0e1734f00f120c4ac7a0e2ebbaa888f11 (patch) | |
| tree | 084fffe314183c2dd1bd3fd975049cbc87fecade | |
| parent | 705ea5b8c29a7ea9d1e2865abeec3d31d4c18d93 (diff) | |
| download | samba-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.c | 39 |
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) |