s3:registry: fix race in reg_setvalue that could lead to data corruption
[ddiss/samba.git] / source3 / registry / reg_api.c
index 743e139a595b97e5089785f548cba8677c385118..bd3cabb86e5f803013e9bda76c38aab3e3bd29b8 100644 (file)
@@ -66,7 +66,6 @@
 #include "registry.h"
 #include "reg_api.h"
 #include "reg_cachehook.h"
-#include "reg_util_internal.h"
 #include "reg_backend_db.h"
 #include "reg_dispatcher.h"
 #include "reg_objects.h"
@@ -90,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);
 
@@ -130,7 +130,7 @@ static int regkey_destructor(struct registry_key_handle *key)
 static WERROR regkey_open_onelevel(TALLOC_CTX *mem_ctx, 
                                   struct registry_key *parent,
                                   const char *name,
-                                  const struct nt_user_token *token,
+                                  const struct security_token *token,
                                   uint32 access_desired,
                                   struct registry_key **pregkey)
 {
@@ -235,7 +235,7 @@ done:
 
 WERROR reg_openhive(TALLOC_CTX *mem_ctx, const char *hive,
                    uint32 desired_access,
-                   const struct nt_user_token *token,
+                   const struct security_token *token,
                    struct registry_key **pkey)
 {
        SMB_ASSERT(hive != NULL);
@@ -376,6 +376,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)
 {
@@ -394,7 +431,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);
                }
        }
 
@@ -659,6 +703,7 @@ done:
 WERROR reg_setvalue(struct registry_key *key, const char *name,
                    const struct registry_value *val)
 {
+       struct regval_blob *existing;
        WERROR err;
        int res;
 
@@ -666,24 +711,62 @@ 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 = 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)));
+               goto done;
+       }
+
+       existing = regval_ctr_getvalue(key->values, name);
+
+       if ((existing != NULL) &&
+           (regval_size(existing) == val->data.length) &&
+           (memcmp(regval_data_p(existing), val->data.data,
+                   val->data.length) == 0))
+       {
+               err = WERR_OK;
+               goto done;
+       }
+
        res = regval_ctr_addvalue(key->values, name, val->type,
                                  val->data.data, val->data.length);
 
        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)
@@ -784,22 +867,15 @@ WERROR reg_deleteallvalues(struct registry_key *key)
  * Note that reg_deletekey returns ACCESS_DENIED when called on a
  * key that has subkeys.
  */
-static WERROR reg_deletekey_recursive_internal(TALLOC_CTX *ctx,
-                                              struct registry_key *parent,
+static WERROR reg_deletekey_recursive_internal(struct registry_key *parent,
                                               const char *path,
                                               bool del_key)
 {
-       TALLOC_CTX *mem_ctx = NULL;
        WERROR werr = WERR_OK;
        struct registry_key *key;
        char *subkey_name = NULL;
        uint32 i;
-
-       mem_ctx = talloc_new(ctx);
-       if (mem_ctx == NULL) {
-               werr = WERR_NOMEM;
-               goto done;
-       }
+       TALLOC_CTX *mem_ctx = talloc_stackframe();
 
        /* recurse through subkeys first */
        werr = reg_openkey(mem_ctx, parent, path, REG_KEY_ALL, &key);
@@ -816,9 +892,7 @@ static WERROR reg_deletekey_recursive_internal(TALLOC_CTX *ctx,
         */
        for (i = regsubkey_ctr_numkeys(key->subkeys) ; i > 0; i--) {
                subkey_name = regsubkey_ctr_specific_key(key->subkeys, i-1);
-               werr = reg_deletekey_recursive_internal(mem_ctx, key,
-                                       subkey_name,
-                                       true);
+               werr = reg_deletekey_recursive_internal(key, subkey_name, true);
                W_ERROR_NOT_OK_GOTO_DONE(werr);
        }
 
@@ -832,8 +906,7 @@ done:
        return werr;
 }
 
-static WERROR reg_deletekey_recursive_trans(TALLOC_CTX *ctx,
-                                           struct registry_key *parent,
+static WERROR reg_deletekey_recursive_trans(struct registry_key *parent,
                                            const char *path,
                                            bool del_key)
 {
@@ -847,17 +920,24 @@ static WERROR reg_deletekey_recursive_trans(TALLOC_CTX *ctx,
                return werr;
        }
 
-       werr = reg_deletekey_recursive_internal(ctx, parent, path, del_key);
+       werr = reg_deletekey_recursive_internal(parent, path, del_key);
 
        if (!W_ERROR_IS_OK(werr)) {
+               WERROR werr2;
+
                DEBUG(1, (__location__ " failed to delete key '%s' from key "
                          "'%s': %s\n", path, parent->key->name,
                          win_errstr(werr)));
-               werr = regdb_transaction_cancel();
-               if (!W_ERROR_IS_OK(werr)) {
+
+               werr2 = regdb_transaction_cancel();
+               if (!W_ERROR_IS_OK(werr2)) {
                        DEBUG(0, ("reg_deletekey_recursive_trans: "
                                  "error cancelling transaction: %s\n",
-                                 win_errstr(werr)));
+                                 win_errstr(werr2)));
+                       /*
+                        * return the original werr or the
+                        * error from cancelling the transaction?
+                        */
                }
        } else {
                werr = regdb_transaction_commit();
@@ -871,17 +951,15 @@ static WERROR reg_deletekey_recursive_trans(TALLOC_CTX *ctx,
        return werr;
 }
 
-WERROR reg_deletekey_recursive(TALLOC_CTX *ctx,
-                              struct registry_key *parent,
+WERROR reg_deletekey_recursive(struct registry_key *parent,
                               const char *path)
 {
-       return reg_deletekey_recursive_trans(ctx, parent, path, true);
+       return reg_deletekey_recursive_trans(parent, path, true);
 }
 
-WERROR reg_deletesubkeys_recursive(TALLOC_CTX *ctx,
-                                  struct registry_key *parent,
+WERROR reg_deletesubkeys_recursive(struct registry_key *parent,
                                   const char *path)
 {
-       return reg_deletekey_recursive_trans(ctx, parent, path, false);
+       return reg_deletekey_recursive_trans(parent, path, false);
 }