s4-drepl: Ensure that the op->source does not get deallocated too early

We need to have the struct dreplsrv_partition_source_dsa around until the end of the
async op, so we use talloc_reference after carefully checking the callers and
making the modifications required.

This prevents a crash when replicating partitions in the vampire_dc test after
adding DNS replication at join time.

Andrew Bartlett

2 files changed, 25 insertions, 9 deletions

diff --git a/source4/dsdb/repl/drepl_extended.c b/source4/dsdb/repl/drepl_extended.c
index 69cccb8409..8735005941 100644
--- a/source4/dsdb/repl/drepl_extended.c
+++ b/source4/dsdb/repl/drepl_extended.c
@@ -39,6 +39,7 @@
   source_dsa_dn: the DN of the server that we are replicating from
  */
 static WERROR drepl_create_extended_source_dsa(struct dreplsrv_service *service,
+					       TALLOC_CTX *mem_ctx,
 					       struct ldb_dn *nc_dn,
 					       struct ldb_dn *source_dsa_dn,
 					       uint64_t min_usn,
@@ -165,7 +166,7 @@ static void extended_op_callback(struct dreplsrv_service *service,
 				 void *cb_data)
 {
 	struct extended_op_data *data = talloc_get_type_abort(cb_data, struct extended_op_data);
-	talloc_free(data->sdsa);
+	talloc_unlink(data, data->sdsa);
 	data->callback(service, err, exop_error, data->callback_data);
 	talloc_free(data);
 }
@@ -184,23 +185,20 @@ WERROR drepl_request_extended_op(struct dreplsrv_service *service,
 {
 	WERROR werr;
 	struct extended_op_data *data;
-	struct dreplsrv_partition_source_dsa *sdsa;
-
-	werr = drepl_create_extended_source_dsa(service, nc_dn, source_dsa_dn, min_usn, &sdsa);
-	W_ERROR_NOT_OK_RETURN(werr);
 
 	data = talloc(service, struct extended_op_data);
 	W_ERROR_HAVE_NO_MEMORY(data);
 
+	werr = drepl_create_extended_source_dsa(service, data, nc_dn, source_dsa_dn, min_usn, &data->sdsa);
+	W_ERROR_NOT_OK_RETURN(werr);
+
 	data->callback = callback;
 	data->callback_data = callback_data;
-	data->sdsa = sdsa;
 
-	werr = dreplsrv_schedule_partition_pull_source(service, sdsa,
+	werr = dreplsrv_schedule_partition_pull_source(service, data->sdsa,
 						       0, extended_op, fsmo_info,
 						       extended_op_callback, data);
 	if (!W_ERROR_IS_OK(werr)) {
-		talloc_free(sdsa);
 		talloc_free(data);
 	}
 
diff --git a/source4/dsdb/repl/drepl_out_pull.c b/source4/dsdb/repl/drepl_out_pull.c
index 86b513d21d..58d877870e 100644
--- a/source4/dsdb/repl/drepl_out_pull.c
+++ b/source4/dsdb/repl/drepl_out_pull.c
@@ -101,7 +101,25 @@ WERROR dreplsrv_schedule_partition_pull_source(struct dreplsrv_service *s,
 	W_ERROR_HAVE_NO_MEMORY(op);
 
 	op->service	= s;
-	op->source_dsa	= source;
+	/*
+	 * source may either be the long-term list of partners, or
+	 * from dreplsrv_partition_source_dsa_temporary().  Because it
+	 * can be either, we can't talloc_steal() it here, so we
+	 * instead we reference it.
+	 *
+	 * We never talloc_free() the p->sources pointers - indeed we
+	 * never remove them - and the temp source will otherwise go
+	 * away with the msg it is allocated on.
+	 *
+	 * Finally the pointer created in drepl_request_extended_op()
+	 * is removed with talloc_unlink().
+	 *
+	 */
+	op->source_dsa	= talloc_reference(op, source);
+	if (!op->source_dsa) {
+		return WERR_NOMEM;
+	}
+
 	op->options	= options;
 	op->extended_op = extended_op;
 	op->fsmo_info   = fsmo_info;