schema: Make writing indices flag an enum for a new state

In schema_load_init, we find that the writing of indices is not locked
in any way. This leads to race conditions. To resolve this, we need to
have a new state (SCHEMA_COMPARE) which can report to the caller that we
need to open a transaction to write the indices.

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index 09623a6d53b7bef4941ec36c9f5a08a354eff605..4d811b009ad9eecd9749f20d9345709f24c2b2e0 100644 (file)
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -919,7 +919,7 @@ static PyObject *py_dsdb_set_schema_from_ldb(PyObject *self, PyObject *args)
        struct ldb_context *from_ldb;
        struct dsdb_schema *schema;
        int ret;
-       char write_indices_and_attributes = true;
+       char write_indices_and_attributes = SCHEMA_WRITE;
        if (!PyArg_ParseTuple(args, "OO|b",
                              &py_ldb, &py_from_ldb, &write_indices_and_attributes))
                return NULL;

diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index d9365ae555351ddfb53765ea8d937c6e1ae71f96..0c44960cf5f3171419ee406ccebbc50f53049f92 100644 (file)
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -867,7 +867,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                cur_schema = dsdb_get_schema(ldb, tmp_ctx);
                used_global_schema = dsdb_uses_global_schema(ldb);
 
-               ret = dsdb_reference_schema(ldb, working_schema, false);
+               ret = dsdb_reference_schema(ldb, working_schema, SCHEMA_MEMORY_ONLY);
                if (ret != LDB_SUCCESS) {
                        DEBUG(0,(__location__ "Failed to reference working schema - %s\n",
                                 ldb_strerror(ret)));
@@ -884,7 +884,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                if (used_global_schema) { 
                        dsdb_set_global_schema(ldb);
                } else if (cur_schema) {
-                       dsdb_reference_schema(ldb, cur_schema, false);
+                       dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                }
 
                if (W_ERROR_EQUAL(objects->error, WERR_DS_DRA_RECYCLED_TARGET)) {
@@ -917,7 +917,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                        if (used_global_schema) { 
                                dsdb_set_global_schema(ldb);
                        } else if (cur_schema ) {
-                               dsdb_reference_schema(ldb, cur_schema, false);
+                               dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                        }
                        DEBUG(0,("Failed to save updated prefixMap: %s\n",
                                 win_errstr(werr)));
@@ -932,7 +932,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                if (used_global_schema) { 
                        dsdb_set_global_schema(ldb);
                } else if (cur_schema ) {
-                       dsdb_reference_schema(ldb, cur_schema, false);
+                       dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                }
                DEBUG(0,(__location__ " Failed to prepare commit of transaction: %s\n",
                         ldb_errstring(ldb)));
@@ -946,7 +946,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                if (used_global_schema) { 
                        dsdb_set_global_schema(ldb);
                } else if (cur_schema ) {
-                       dsdb_reference_schema(ldb, cur_schema, false);
+                       dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                }
                DEBUG(0,(__location__ " Failed to load partition uSN\n"));
                ldb_transaction_cancel(ldb);
@@ -960,7 +960,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                if (used_global_schema) { 
                        dsdb_set_global_schema(ldb);
                } else if (cur_schema ) {
-                       dsdb_reference_schema(ldb, cur_schema, false);
+                       dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                }
                DEBUG(0,(__location__ " Failed to commit transaction\n"));
                TALLOC_FREE(tmp_ctx);
@@ -1005,7 +1005,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
                                new_schema != NULL ?
                                new_schema->metadata_usn : 0,
                                working_schema, working_schema->metadata_usn);
-                       dsdb_reference_schema(ldb, cur_schema, false);
+                       dsdb_reference_schema(ldb, cur_schema, SCHEMA_MEMORY_ONLY);
                        if (used_global_schema) {
                                dsdb_set_global_schema(ldb);
                        }

diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index b3313b45d6b4c5dbe16290f39e7d87861e0d3a9c..4013cdf6364ccd5a1d3682a54f702b69c42dad14 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -246,7 +246,7 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
                return schema;
        }
 
-       ret = dsdb_set_schema(ldb, new_schema, false);
+       ret = dsdb_set_schema(ldb, new_schema, SCHEMA_MEMORY_ONLY);
        if (ret != LDB_SUCCESS) {
                ldb_debug_set(ldb, LDB_DEBUG_FATAL,
                              "dsdb_set_schema() failed: %d:%s: %s",
@@ -359,7 +359,8 @@ failed:
 }      
 
 static int schema_load(struct ldb_context *ldb,
-                      struct ldb_module *module)
+                      struct ldb_module *module,
+                      bool *need_write)
 {
        struct dsdb_schema *schema;
        void *readOnlySchema;
@@ -406,7 +407,7 @@ static int schema_load(struct ldb_context *ldb,
                }
 
                /* "dsdb_set_schema()" steals schema into the ldb_context */
-               ret = dsdb_set_schema(ldb, new_schema, false);
+               ret = dsdb_set_schema(ldb, new_schema, SCHEMA_MEMORY_ONLY);
                if (ret != LDB_SUCCESS) {
                        ldb_debug_set(ldb, LDB_DEBUG_FATAL,
                                      "schema_load_init: dsdb_set_schema() failed: %d:%s: %s",
@@ -444,7 +445,14 @@ static int schema_load(struct ldb_context *ldb,
 
        /* Now check the @INDEXLIST is correct, or fix it up */
        ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
-                                                    true);
+                                                    SCHEMA_COMPARE);
+       if (ret == LDB_ERR_BUSY) {
+               *need_write = true;
+               ret = LDB_SUCCESS;
+       } else {
+               *need_write = false;
+       }
+
        if (ret != LDB_SUCCESS) {
                ldb_asprintf_errstring(ldb, "Failed to update "
                                       "@INDEXLIST and @ATTRIBUTES "
@@ -463,6 +471,7 @@ static int schema_load_init(struct ldb_module *module)
        struct ldb_context *ldb = ldb_module_get_ctx(module);
        struct schema_load_private_data *private_data;
        int ret;
+       bool need_write = false;
 
        private_data = talloc_zero(module, struct schema_load_private_data);
        if (private_data == NULL) {
@@ -477,11 +486,49 @@ static int schema_load_init(struct ldb_module *module)
                return ret;
        }
 
-       ret = schema_load(ldb, module);
-       if (ret != LDB_SUCCESS) {
-               return ret;
+       ret = schema_load(ldb, module, &need_write);
+
+       if (ret == LDB_SUCCESS && need_write) {
+               TALLOC_CTX *frame = talloc_stackframe();
+               struct dsdb_schema *schema = NULL;
+
+               ret = ldb_transaction_start(ldb);
+               if (ret != LDB_SUCCESS) {
+                       ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+                                     "schema_load_init: transaction start failed");
+                       return LDB_ERR_OPERATIONS_ERROR;
+               }
+
+               schema = dsdb_get_schema(ldb, frame);
+               if (schema == NULL) {
+                       ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+                                     "schema_load_init: dsdb_get_schema failed");
+                       ldb_transaction_cancel(ldb);
+                       return LDB_ERR_OPERATIONS_ERROR;
+               }
+               ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
+                                                            SCHEMA_WRITE);
+
+               TALLOC_FREE(frame);
+
+               if (ret != LDB_SUCCESS) {
+                       ldb_asprintf_errstring(ldb, "Failed to write new "
+                                              "@INDEXLIST and @ATTRIBUTES "
+                                              "records for updated schema: %s",
+                                              ldb_errstring(ldb));
+                       ldb_transaction_cancel(ldb);
+                       return ret;
+               }
+
+               ret = ldb_transaction_commit(ldb);
+               if (ret != LDB_SUCCESS) {
+                       ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+                                     "schema_load_init: transaction commit failed");
+                       return LDB_ERR_OPERATIONS_ERROR;
+               }
        }
 
+
        return ret;
 }
 
@@ -549,7 +596,7 @@ static int schema_load_extended(struct ldb_module *module, struct ldb_request *r
 
        ret = dsdb_schema_set_indices_and_attributes(ldb,
                                                     schema,
-                                                    true);
+                                                    SCHEMA_WRITE);
 
        if (ret != LDB_SUCCESS) {
                ldb_asprintf_errstring(ldb, "Failed to write new "

diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h
index 75e4886b0279bfbdfb58e09399d47db906a23e9f..9b27fd24b64079caefb6afc45feb76d3dedf3c2d 100644 (file)
--- a/source4/dsdb/schema/schema.h
+++ b/source4/dsdb/schema/schema.h
@@ -192,6 +192,12 @@ struct dsdb_class {
        } tmp;
 };
 
+enum schema_set_enum {
+       SCHEMA_MEMORY_ONLY = 0,
+       SCHEMA_WRITE = 1,
+       SCHEMA_COMPARE = 2,
+};
+
 /**
  * data stored in schemaInfo attribute
  */

diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index ca7a307212a3db06897b6c5a6044483da10b7a80..32cdd7192d68b27b32c40c3b117ab1b4b68dc986 100644 (file)
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -58,7 +58,7 @@ const struct ldb_schema_attribute *dsdb_attribute_handler_override(struct ldb_co
  */
 int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
                                           struct dsdb_schema *schema,
-                                          bool write_indices_and_attributes)
+                                          enum schema_set_enum mode)
 {
        int ret = LDB_SUCCESS;
        struct ldb_result *res;
@@ -80,7 +80,7 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
                ldb_schema_set_override_GUID_index(ldb, "objectGUID", "GUID");
        }
 
-       if (!write_indices_and_attributes) {
+       if (mode == SCHEMA_MEMORY_ONLY) {
                return ret;
        }
 
@@ -178,9 +178,17 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
        ret = ldb_search(ldb, mem_ctx, &res, msg->dn, LDB_SCOPE_BASE, NULL,
                         NULL);
        if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+               if (mode == SCHEMA_COMPARE) {
+                       /* We are probably not in a transaction */
+                       goto wrong_mode;
+               }
                ret = ldb_add(ldb, msg);
        } else if (ret != LDB_SUCCESS) {
        } else if (res->count != 1) {
+               if (mode == SCHEMA_COMPARE) {
+                       /* We are probably not in a transaction */
+                       goto wrong_mode;
+               }
                ret = ldb_add(ldb, msg);
        } else {
                ret = LDB_SUCCESS;
@@ -198,6 +206,10 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
                         * are under the read lock and we wish to do a
                         * delete of any removed/renamed attributes
                         */
+                       if (mode == SCHEMA_COMPARE) {
+                               /* We are probably not in a transaction */
+                               goto wrong_mode;
+                       }
                        ret = dsdb_modify(ldb, mod_msg, 0);
                }
                talloc_free(mod_msg);
@@ -219,9 +231,17 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
        ret = ldb_search(ldb, mem_ctx, &res_idx, msg_idx->dn, LDB_SCOPE_BASE,
                         NULL, NULL);
        if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+               if (mode == SCHEMA_COMPARE) {
+                       /* We are probably not in a transaction */
+                       goto wrong_mode;
+               }
                ret = ldb_add(ldb, msg_idx);
        } else if (ret != LDB_SUCCESS) {
        } else if (res_idx->count != 1) {
+               if (mode == SCHEMA_COMPARE) {
+                       /* We are probably not in a transaction */
+                       goto wrong_mode;
+               }
                ret = ldb_add(ldb, msg_idx);
        } else {
                ret = LDB_SUCCESS;
@@ -260,6 +280,10 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
                         * are under the read lock and we wish to do a
                         * delete of any removed/renamed attributes
                         */
+                       if (mode == SCHEMA_COMPARE) {
+                               /* We are probably not in a transaction */
+                               goto wrong_mode;
+                       }
                        ret = dsdb_modify(ldb, mod_msg, 0);
                }
                talloc_free(mod_msg);
@@ -280,6 +304,10 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
 op_error:
        talloc_free(mem_ctx);
        return ldb_operr(ldb);
+
+wrong_mode:
+       talloc_free(mem_ctx);
+       return LDB_ERR_BUSY;
 }
 
 
@@ -531,7 +559,7 @@ int dsdb_set_schema_refresh_function(struct ldb_context *ldb,
  */
 int dsdb_set_schema(struct ldb_context *ldb,
                    struct dsdb_schema *schema,
-                   bool write_indices_and_attributes)
+                   enum schema_set_enum write_indices_and_attributes)
 {
        struct dsdb_schema *old_schema;
        int ret;
@@ -586,7 +614,7 @@ static struct dsdb_schema *global_schema;
  * disk.
  */
 int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
-                         bool write_indices_and_attributes)
+                         enum schema_set_enum write_indices_and_attributes)
 {
        int ret;
        struct dsdb_schema *old_schema;
@@ -657,7 +685,8 @@ int dsdb_set_global_schema(struct ldb_context *ldb)
        talloc_unlink(ldb, old_schema);
 
        /* Set the new attributes based on the new schema */
-       ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, false /* Don't write indices and attributes, it's expensive */);
+       /* Don't write indices and attributes, it's expensive */
+       ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, SCHEMA_MEMORY_ONLY);
        if (ret == LDB_SUCCESS) {
                /* Keep a reference to this schema, just in case the original copy is replaced */
                if (talloc_reference(ldb, global_schema) == NULL) {
@@ -948,7 +977,7 @@ WERROR dsdb_set_schema_from_ldif(struct ldb_context *ldb,
                }
        }
 
-       ret = dsdb_set_schema(ldb, schema, true);
+       ret = dsdb_set_schema(ldb, schema, SCHEMA_WRITE);
        if (ret != LDB_SUCCESS) {
                status = WERR_FOOBAR;
                DEBUG(0,("ERROR: dsdb_set_schema() failed with %s / %s\n",

diff --git a/source4/dsdb/schema/tests/schema_syntax.c b/source4/dsdb/schema/tests/schema_syntax.c
index cb4d8029546eed1b6e6e4993fba76bb3f3957c3e..17e62010e70d3f0f805ea55f390d090a3d036403 100644 (file)
--- a/source4/dsdb/schema/tests/schema_syntax.c
+++ b/source4/dsdb/schema/tests/schema_syntax.c
@@ -78,7 +78,7 @@ static bool torture_syntax_add_OR_Name(struct torture_context *tctx,
        ldb_ldif_read_free(ldb, ldif);
        torture_assert_werr_ok(tctx, werr, "dsdb_set_attribute_from_ldb() failed!");
 
-       ldb_res = dsdb_set_schema(ldb, schema, true);
+       ldb_res = dsdb_set_schema(ldb, schema, SCHEMA_WRITE);
        torture_assert_int_equal(tctx, ldb_res, LDB_SUCCESS, "dsdb_set_schema() failed");
 
        return true;

diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
index a449e1c86efef988a0a0baf0f572c2422ea6394f..2871a683b74d9844f298e1b244d5531cfd20189e 100644 (file)
--- a/source4/libnet/libnet_vampire.c
+++ b/source4/libnet/libnet_vampire.c
@@ -331,7 +331,7 @@ static WERROR libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s,
                provision_schema = dsdb_get_schema(s->ldb, s);
        } else {
                provision_schema = dsdb_get_schema(schema_ldb, s);
-               ret = dsdb_reference_schema(s->ldb, provision_schema, false);
+               ret = dsdb_reference_schema(s->ldb, provision_schema, SCHEMA_MEMORY_ONLY);
                if (ret != LDB_SUCCESS) {
                        DEBUG(0,("Failed to attach schema from local provision using remote prefixMap."));
                        return WERR_INTERNAL_ERROR;
@@ -368,7 +368,7 @@ static WERROR libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s,
         * attach the schema we just brought over DRS to the ldb,
         * so we can use it in dsdb_convert_object_ex below
         */
-       ret = dsdb_set_schema(s->ldb, s->self_made_schema, true);
+       ret = dsdb_set_schema(s->ldb, s->self_made_schema, SCHEMA_WRITE);
        if (ret != LDB_SUCCESS) {
                DEBUG(0,("Failed to attach working schema from DRS.\n"));
                return WERR_INTERNAL_ERROR;

diff --git a/source4/torture/drs/drs_util.c b/source4/torture/drs/drs_util.c
index 7c073c9f6e73425737ab6b477dd810e73805dbb6..4795bd90fe32010370e27f6d78206e47098c349a 100644 (file)
--- a/source4/torture/drs/drs_util.c
+++ b/source4/torture/drs/drs_util.c
@@ -158,7 +158,7 @@ bool drs_util_dsdb_schema_load_ldb(struct torture_context *tctx,
 
        talloc_free(res);
 
-       ret = dsdb_set_schema(ldb, ldap_schema, true);
+       ret = dsdb_set_schema(ldb, ldap_schema, SCHEMA_WRITE);
        if (ret != LDB_SUCCESS) {
                torture_fail(tctx,
                             talloc_asprintf(tctx, "dsdb_set_schema() failed: %s", ldb_strerror(ret)));