samdb/schema_load: do schema loading with one search
authorBob Campbell <bobcampbell@catalyst.net.nz>
Tue, 11 Jul 2017 04:40:14 +0000 (16:40 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Mon, 5 Mar 2018 19:50:15 +0000 (20:50 +0100)
It appears that there was a race condition between searching for the
attribute & class definitions, and searching for the schema object, if
the schema was changed in-between the two searches.

This is likely the cause of ldap_schema being flapping.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12889

Signed-off-by: Bob Campbell <bobcampbell@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
source4/dsdb/samdb/ldb_modules/schema_load.c
source4/dsdb/schema/schema_init.c

index f1a01d73f5fb2a8f2620c33295f2e89a42e10626..2d712e2c6df23079b44db10ccf464f55c8fcd8ef 100644 (file)
@@ -296,20 +296,17 @@ static int dsdb_schema_from_db(struct ldb_module *module,
        struct ldb_context *ldb = ldb_module_get_ctx(module);
        TALLOC_CTX *tmp_ctx;
        char *error_string;
-       int ret;
+       int ret, i;
        struct ldb_dn *schema_dn = ldb_get_schema_basedn(ldb);
-       struct ldb_result *schema_res;
        struct ldb_result *res;
-       static const char *schema_head_attrs[] = {
-               "prefixMap",
-               "schemaInfo",
-               "fSMORoleOwner",
-               NULL
-       };
+       struct ldb_message *schema_msg = NULL;
        static const char *schema_attrs[] = {
                DSDB_SCHEMA_COMMON_ATTRS,
                DSDB_SCHEMA_ATTR_ATTRS,
                DSDB_SCHEMA_CLASS_ATTRS,
+               "prefixMap",
+               "schemaInfo",
+               "fSMORoleOwner",
                NULL
        };
        unsigned flags;
@@ -324,33 +321,20 @@ static int dsdb_schema_from_db(struct ldb_module *module,
        ldb_set_flags(ldb, flags & ~LDB_FLG_ENABLE_TRACING);
 
        /*
-        * setup the prefix mappings and schema info
-        */
-       ret = dsdb_module_search_dn(module, tmp_ctx, &schema_res,
-                                   schema_dn, schema_head_attrs,
-                                   DSDB_FLAG_NEXT_MODULE, NULL);
-       if (ret == LDB_ERR_NO_SUCH_OBJECT) {
-               ldb_reset_err_string(ldb);
-               ldb_debug(ldb, LDB_DEBUG_WARNING,
-                         "schema_load_init: no schema head present: (skip schema loading)\n");
-               goto failed;
-       } else if (ret != LDB_SUCCESS) {
-               ldb_asprintf_errstring(ldb, 
-                                      "dsdb_schema: failed to search the schema head: %s",
-                                      ldb_errstring(ldb));
-               goto failed;
-       }
-
-       /*
-        * load the attribute definitions.
+        * Load the attribute and class definitions, as well as
+        * the schema object. We do this in one search and then
+        * split it so that there isn't a race condition when
+        * the schema is changed between two searches.
         */
        ret = dsdb_module_search(module, tmp_ctx, &res,
-                                schema_dn, LDB_SCOPE_ONELEVEL,
+                                schema_dn, LDB_SCOPE_SUBTREE,
                                 schema_attrs,
                                 DSDB_FLAG_NEXT_MODULE |
                                 DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
                                 NULL,
-                                "(|(objectClass=attributeSchema)(objectClass=classSchema))");
+                                "(|(objectClass=attributeSchema)"
+                                "(objectClass=classSchema)"
+                                "(objectClass=dMD))");
        if (ret != LDB_SUCCESS) {
                ldb_asprintf_errstring(ldb, 
                                       "dsdb_schema: failed to search attributeSchema and classSchema objects: %s",
@@ -358,8 +342,26 @@ static int dsdb_schema_from_db(struct ldb_module *module,
                goto failed;
        }
 
+       /*
+        * Separate the schema object from the attribute and
+        * class objects.
+        */
+       for (i = 0; i < res->count; i++) {
+               if (ldb_msg_find_element(res->msgs[i], "prefixMap")) {
+                       schema_msg = res->msgs[i];
+                       break;
+               }
+       }
+
+       if (schema_msg == NULL) {
+               ldb_asprintf_errstring(ldb,
+                                      "dsdb_schema load failed: failed to find prefixMap");
+               ret = LDB_ERR_NO_SUCH_ATTRIBUTE;
+               goto failed;
+       }
+
        ret = dsdb_schema_from_ldb_results(tmp_ctx, ldb,
-                                          schema_res, res, schema, &error_string);
+                                          schema_msg, res, schema, &error_string);
        if (ret != LDB_SUCCESS) {
                ldb_asprintf_errstring(ldb, 
                                       "dsdb_schema load failed: %s",
index dbd504549d72563aafad65026f62b5b1b4fcc135..9314ce731b8b6efa11b8995928a7323d06bd8551 100644 (file)
@@ -904,10 +904,24 @@ int dsdb_load_ldb_results_into_schema(TALLOC_CTX *mem_ctx, struct ldb_context *l
                                      char **error_string)
 {
        unsigned int i;
+       WERROR status;
 
        schema->ts_last_change = 0;
        for (i=0; i < attrs_class_res->count; i++) {
-               WERROR status = dsdb_schema_set_el_from_ldb_msg(ldb, schema, attrs_class_res->msgs[i]);
+               const char *prefixMap = NULL;
+               /*
+                * attrs_class_res also includes the schema object;
+                * we only want to process classes & attributes
+                */
+               prefixMap = ldb_msg_find_attr_as_string(
+                               attrs_class_res->msgs[i],
+                               "prefixMap", NULL);
+               if (prefixMap != NULL) {
+                       continue;
+               }
+
+               status = dsdb_schema_set_el_from_ldb_msg(ldb, schema,
+                                                        attrs_class_res->msgs[i]);
                if (!W_ERROR_IS_OK(status)) {
                        *error_string = talloc_asprintf(mem_ctx,
                                      "dsdb_load_ldb_results_into_schema: failed to load attribute or class definition: %s:%s",
@@ -928,7 +942,7 @@ int dsdb_load_ldb_results_into_schema(TALLOC_CTX *mem_ctx, struct ldb_context *l
 */
 
 int dsdb_schema_from_ldb_results(TALLOC_CTX *mem_ctx, struct ldb_context *ldb,
-                                struct ldb_result *schema_res,
+                                struct ldb_message *schema_msg,
                                 struct ldb_result *attrs_class_res,
                                 struct dsdb_schema **schema_out,
                                 char **error_string)
@@ -961,7 +975,7 @@ int dsdb_schema_from_ldb_results(TALLOC_CTX *mem_ctx, struct ldb_context *ldb,
                                                              false);
        }
 
-       prefix_val = ldb_msg_find_ldb_val(schema_res->msgs[0], "prefixMap");
+       prefix_val = ldb_msg_find_ldb_val(schema_msg, "prefixMap");
        if (!prefix_val) {
                *error_string = talloc_asprintf(mem_ctx, 
                                                "schema_fsmo_init: no prefixMap attribute found");
@@ -969,7 +983,7 @@ int dsdb_schema_from_ldb_results(TALLOC_CTX *mem_ctx, struct ldb_context *ldb,
                talloc_free(tmp_ctx);
                return LDB_ERR_CONSTRAINT_VIOLATION;
        }
-       info_val = ldb_msg_find_ldb_val(schema_res->msgs[0], "schemaInfo");
+       info_val = ldb_msg_find_ldb_val(schema_msg, "schemaInfo");
        if (!info_val) {
                status = dsdb_schema_info_blob_new(mem_ctx, &info_val_default);
                if (!W_ERROR_IS_OK(status)) {
@@ -999,7 +1013,7 @@ int dsdb_schema_from_ldb_results(TALLOC_CTX *mem_ctx, struct ldb_context *ldb,
                return ret;
        }
 
-       schema->fsmo.master_dn = ldb_msg_find_attr_as_dn(ldb, schema, schema_res->msgs[0], "fSMORoleOwner");
+       schema->fsmo.master_dn = ldb_msg_find_attr_as_dn(ldb, schema, schema_msg, "fSMORoleOwner");
        if (ldb_dn_compare(samdb_ntds_settings_dn(ldb, tmp_ctx), schema->fsmo.master_dn) == 0) {
                schema->fsmo.we_are_master = true;
        } else {