autobuild failure due to python replication tests - why ?

Jeremy Allison jra at samba.org
Thu Aug 4 18:24:18 UTC 2016


On Thu, Aug 04, 2016 at 08:46:27AM +0200, Stefan Metzmacher wrote:
> Hi Andrew,
> 
> I think I got it.
> 
> I the testenv I'm doing:
> 
> for i in $(seq 1 2); do
>         echo $i;
>         PYTHONPATH=bin/python:source4/torture/drs/python DC1=$DC_SERVER
> DC2=$VAMPIRE_DC_SERVER source4/scripting/bin/subunitrun repl_schema
> -U$DOMAIN/$DC_USERNAME%$DC_PASSWORD || {
>                 echo "FAIL DC1 DC2: $i"; break;
>         } ;
>         PYTHONPATH=bin/python:source4/torture/drs/python DC2=$DC_SERVER
> DC1=$VAMPIRE_DC_SERVER source4/scripting/bin/subunitrun repl_schema
> -U$DOMAIN/$DC_USERNAME%$DC_PASSWORD || {
>                 echo "FAIL DC2 DC1: $i"; break;
>         } ;
>         echo "OK: $i";
> done
> 
> I added some debug messages to dsdb_schema_info_cmp().
> 
> I got a loop like this on one dc:
> 
> remote[FF00000011E040FDD29D832940A43153F1711A0E3A]
> local[FF00000012ED817478D89B2B46AB425900C898C97F]
> ts_last_change[Thu Aug  4 08:34:20 2016 CEST], metadata_usn[8]
> Can't replicate DC=samba,DC=example,DC=com because remote schema has
> changed since we last replicated the schema
> Wrong schema when applying reply GetNCChanges, retrying
> Replicated 0 objects (0 linked attributes) for
> CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com
> dsdb_schema_info_cmp(WERR_DS_DRA_SCHEMA_MISMATCH):
> remote[FF00000011E040FDD29D832940A43153F1711A0E3A]
> local[FF00000012ED817478D89B2B46AB425900C898C97F]
> ts_last_change[Thu Aug  4 08:34:20 2016 CEST], metadata_usn[8]
> Can't replicate DC=samba,DC=example,DC=com because remote schema has
> changed since we last replicated the schema
> Wrong schema when applying reply GetNCChanges, retrying
> Replicated 0 objects (0 linked attributes) for
> CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com
> dsdb_schema_info_cmp(WERR_DS_DRA_SCHEMA_MISMATCH):
> remote[FF00000011E040FDD29D832940A43153F1711A0E3A]
> local[FF00000012ED817478D89B2B46AB425900C898C97F]
> ts_last_change[Thu Aug  4 08:34:20 2016 CEST], metadata_usn[8]
> Can't replicate DC=samba,DC=example,DC=com because remote schema has
> changed since we last replicated the schema
> Wrong schema when applying reply GetNCChanges, retrying

Metze, what about the following fixes ?

I think the first one is a genuine bugfix as otherwise
there are paths through dreplsrv_op_pull_source_get_changes_trigger()
that can repeatably error without doing a tevent_req_nterror().

The second fix prevents dreplsrv_op_pull_source_get_changes_trigger()
from infinitely recursing via:

dreplsrv_op_pull_source_send()->
    dreplsrv_op_pull_source_connect_done()->
        dreplsrv_op_pull_source_get_changes_trigger()->
    -->     dcerpc_drsuapi_DsGetNCChanges_r_send()->
    |           dreplsrv_op_pull_source_get_changes_done()->
    |               dreplsrv_op_pull_source_apply_changes_trigger()->
    |                   dreplsrv_op_pull_source_get_changes_trigger() <-|
    |___________________________________________________________________|

It's not elegant but something similar is definitely needed here to
stop the potential recursion.

Jeremy.
-------------- next part --------------
From a58d4eb3cd62156291468d648d23ff473d0202e8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 4 Aug 2016 11:09:21 -0700
Subject: [PATCH 1/2] s4: repl: Ensure all error paths in
 dreplsrv_op_pull_source_get_changes_trigger() are protected with tevent
 returns.

Otherwise dreplsrv_op_pull_source_get_changes_trigger() could infinitely recurse.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/dsdb/repl/drepl_out_helpers.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
index bf8a372..29bb9d5 100644
--- a/source4/dsdb/repl/drepl_out_helpers.c
+++ b/source4/dsdb/repl/drepl_out_helpers.c
@@ -446,6 +446,9 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
 		if (!W_ERROR_IS_OK(werr)) {
 			DEBUG(0,(__location__ ": Failed to convert UDV for %s : %s\n",
 				 ldb_dn_get_linearized(partition->dn), win_errstr(werr)));
+			/* The above error can only be OUT_OF_MEMORY. */
+			tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
+			return;
 		}
 	}
 
@@ -470,6 +473,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
 		status = dreplsrv_get_gc_partial_attribute_set(service, r, &pas);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0,(__location__ ": Failed to construct GC partial attribute set : %s\n", nt_errstr(status)));
+			tevent_req_nterror(req, status);
 			return;
 		}
 		replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
@@ -482,6 +486,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
 		status = dreplsrv_get_rodc_partial_attribute_set(service, r, &pas, for_schema);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0,(__location__ ": Failed to construct RODC partial attribute set : %s\n", nt_errstr(status)));
+			tevent_req_nterror(req, status);
 			return;
 		}
 		replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
-- 
2.8.0.rc3.226.g39d4020


From 62c8933b04b8b2d691279b46d2dfc55e4d0c9c87 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 4 Aug 2016 11:13:50 -0700
Subject: [PATCH 2/2] s4: repl: Add protection to
 dreplsrv_op_pull_source_get_changes_trigger() to prevent recursion.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/dsdb/repl/drepl_out_helpers.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
index 29bb9d5..3f34d76 100644
--- a/source4/dsdb/repl/drepl_out_helpers.c
+++ b/source4/dsdb/repl/drepl_out_helpers.c
@@ -241,6 +241,7 @@ struct dreplsrv_op_pull_source_state {
 	struct tevent_context *ev;
 	struct dreplsrv_out_operation *op;
 	void *ndr_struct_ptr;
+	unsigned int depth;
 };
 
 static void dreplsrv_op_pull_source_connect_done(struct tevent_req *subreq);
@@ -260,6 +261,7 @@ struct tevent_req *dreplsrv_op_pull_source_send(TALLOC_CTX *mem_ctx,
 	}
 	state->ev = ev;
 	state->op = op;
+	state->depth = 0;
 
 	subreq = dreplsrv_out_drsuapi_send(state, ev, op->source_dsa->conn);
 	if (tevent_req_nomem(subreq, req)) {
@@ -421,6 +423,19 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
 	struct drsuapi_DsReplicaHighWaterMark highwatermark;
 	struct ldb_dn *schema_dn = ldb_get_schema_basedn(service->samdb);
 
+	/*
+	 * Stop this infinitely recursing.
+	 */
+
+	if (state->depth > 2) {
+		DBG_ERR("called with depth %u. Error out to stop recursion.\n",
+			state->depth);
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+
+	state->depth++;
+
 	r = talloc(state, struct drsuapi_DsGetNCChanges);
 	if (tevent_req_nomem(r, req)) {
 		return;
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list