[PATCH]dsdb partition.c make partition_copy_all async.

Gary Lockyer gary at catalyst.net.nz
Tue May 22 03:41:27 UTC 2018


When updating a special DN partition_copy_all uses ldb_wait to wait for
the update to the primary partition to complete. If a module higher

up the chain inserts a callback, the code blocks in ldb_wait and does

not complete.

This change replaces the ldb_wait logic with a callback.  There is no
code currently triggering this code path.  But the dsdb and group change
auditing do exercise this path and trigger the bug.

Review and push appreciated.

Gary




-------------- next part --------------
From f6722b481a0e5cbd6aa87776ac9326a0331451d6 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon, 21 May 2018 14:31:57 +1200
Subject: [PATCH] dsdb partition.c: Make partition_copy_all aysnc.

partition_copy_all uses ldb_wait to wait for the update to the primary
partition to complete, when updating a special dn.  If a module higher
up the chain inserts a callback, the code blocks in ldb_wait and does
not complete.  This change replaces the ldb_wait logic with a callback.

Currently there is no code that triggers this bug, however the up coming
audit logging changes do trigger this bug.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/partition.c | 200 ++++++++++++++++++++++++++---
 1 file changed, 179 insertions(+), 21 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 9fb1b9d..0068571 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -394,32 +394,37 @@ static int partition_send_all(struct ldb_module *module,
 	return partition_call_first(ac);
 }
 
+struct partition_copy_context {
+	struct ldb_module *module;
+	struct partition_context *partition_context;
+	struct ldb_request *request;
+	struct ldb_dn *dn;
+};
 
-/**
- * send an operation to the top partition, then copy the resulting
- * object to all other partitions
+/*
+ * A special DN has been updated in the primary partition. Now propagate those
+ * changes to the remaining partitions.
+ *
+ * Note: that the operations are asyncchonous and this fuction is called
+ *       from partition_copy_all_callback_handler in response to an async
+ *       callback.
  */
-static int partition_copy_all(struct ldb_module *module,
-			      struct partition_context *ac,
-			      struct ldb_request *req,
-			      struct ldb_dn *dn)
+static int partition_copy_all_callback_action(
+	struct ldb_module *module,
+	struct partition_context *ac,
+	struct ldb_request *req,
+	struct ldb_dn *dn,
+	int ret)
+
 {
+
 	unsigned int i;
-	struct partition_private_data *data = talloc_get_type(ldb_module_get_private(module),
-							      struct partition_private_data);
-	int ret, search_ret;
+	struct partition_private_data *data =
+		talloc_get_type(
+			ldb_module_get_private(module),
+			struct partition_private_data);
+	int search_ret;
 	struct ldb_result *res;
-
-	/* do the request on the top level sam.ldb synchronously */
-	ret = ldb_next_request(module, req);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-	ret = ldb_wait(req->handle, LDB_WAIT_ALL);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
 	/* now fetch the resulting object, and then copy it to all the
 	 * other partitions. We need this approach to cope with the
 	 * partitions getting out of sync. If for example the
@@ -521,6 +526,159 @@ static int partition_copy_all(struct ldb_module *module,
 	return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
 }
 
+
+/*
+ * @brief call back function for the ldb operations on special DN's.
+ *
+ * As the LDB operations are async, and we wish to use the result
+ * the operations, a callback needs to be registered to process the results
+ * of the LDB operations.
+ *
+ * @param req the ldb request
+ * @param res the result of the operation
+ *
+ * @return the LDB_STATUS
+ */
+static int partition_copy_all_callback_handler(
+	struct ldb_request *req,
+	struct ldb_reply *ares)
+{
+	struct partition_copy_context *ac = NULL;
+	int error = ares->error;
+
+	ac = talloc_get_type(
+		req->context,
+		struct partition_copy_context);
+
+	if (!ares) {
+		return ldb_module_done(
+			ac->request,
+			NULL,
+			NULL,
+			LDB_ERR_OPERATIONS_ERROR);
+	}
+
+	/* pass on to the callback */
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+		return ldb_module_send_entry(
+			ac->request,
+			ares->message,
+			ares->controls);
+
+	case LDB_REPLY_REFERRAL:
+		return ldb_module_send_referral(
+			ac->request,
+			ares->referral);
+
+	case LDB_REPLY_DONE:
+		error = partition_copy_all_callback_action(
+			ac->module,
+			ac->partition_context,
+			ac->request,
+			ac->dn,
+			ares->error);
+		return ldb_module_done(
+			ac->request,
+			ares->controls,
+			ares->response,
+			error);
+
+	default:
+		/* Can't happen */
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+}
+
+/**
+ * send an operation to the top partition, then copy the resulting
+ * object to all other partitions.
+ */
+static int partition_copy_all(
+	struct ldb_module *module,
+	struct partition_context *partition_context,
+	struct ldb_request *req,
+	struct ldb_dn *dn)
+{
+	struct ldb_request *new_req = NULL;
+	struct ldb_context *ldb = NULL;
+	struct partition_copy_context *context = NULL;
+
+	int ret;
+
+	ldb = ldb_module_get_ctx(module);
+
+	context = talloc_zero(req, struct partition_copy_context);
+	if (context == NULL) {
+		return ldb_oom(ldb);
+	}
+	context->module = module;
+	context->request = req;
+	context->dn = dn;
+	context->partition_context = partition_context;
+
+	switch (req->operation) {
+	case LDB_ADD:
+		ret = ldb_build_add_req(
+			&new_req,
+			ldb,
+			req,
+			req->op.add.message,
+			req->controls,
+			context,
+			partition_copy_all_callback_handler,
+			req);
+		break;
+	case LDB_MODIFY:
+		ret = ldb_build_mod_req(
+			&new_req,
+			ldb,
+			req,
+			req->op.mod.message,
+			req->controls,
+			context,
+			partition_copy_all_callback_handler,
+			req);
+		break;
+	case LDB_DELETE:
+		ret = ldb_build_del_req(
+			&new_req,
+			ldb,
+			req,
+			req->op.del.dn,
+			req->controls,
+			context,
+			partition_copy_all_callback_handler,
+			req);
+		break;
+	case LDB_RENAME:
+		ret = ldb_build_rename_req(
+			&new_req,
+			ldb,
+			req,
+			req->op.rename.olddn,
+			req->op.rename.newdn,
+			req->controls,
+			context,
+			partition_copy_all_callback_handler,
+			req);
+		break;
+	default:
+		/*
+		 * Shouldn't happen.
+		 */
+		ldb_debug(
+			ldb,
+			LDB_DEBUG_ERROR,
+			"Unexpected opertation type (%d)\n", req->operation);
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		break;
+	}
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+	return ldb_next_request(module, new_req);
+}
 /**
  * Figure out which backend a request needs to be aimed at.  Some
  * requests must be replicated to all backends
-- 
2.7.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180522/cce34c29/signature.sig>


More information about the samba-technical mailing list