[PATCH] Address some -O3 issues on Ubuntu 10.04

[Jeremy Allison]

On Thu, Aug 04, 2016 at 09:29:13PM +0200, Michael Adam wrote:
> On 2016-08-04 at 08:56 -0700, Jeremy Allison wrote:
> > On Thu, Aug 04, 2016 at 11:38:33AM +0200, Michael Adam wrote:
> > > On 2016-08-04 at 00:39 +0200, Michael Adam wrote:
> > > > On 2016-08-03 at 17:41 +0200, Stefan Metzmacher wrote:
> > > > > Hi,
> > > > > 
> > > > > here's an update that also includes andrew's backupkey test fixes.
> > > > > 
> > > > > All with bug references for the backports.
> > > > > 
> > > > > Please review and push.
> > > > > 
> > > > > (I already pushed the two patches from Andrew)
> > > > 
> > > > Reviewed-by: me and pushed to autobuild.
> > > 
> > > Re-pushed after failed autobuild.
> > 
> > Yes, mine and Andreas's autobuild failed also.
> > I'll re-push one more time. Ralph seemed to
> > get something through (god knows how :-).
> 
> Yeah... some time ago. ;-)
> 
> Metze's last autobuild failed as well.
> 
> Mine is currently running (at this moment at the drs tests...)
> and it has metze's last autobuild content included.
> Andreas' autobuild is in the queue mirroring mine.
> 
> > I think if metze recent work doesn't fix it the
> > samba4.*samba_tool* tests need to be skipped
> > until fixed.
> 
> This!

Well after metze's analysis I have one final attempt
at fixing them (attached). I think patch #1 is a
genuine bugfix that needs to go in regardless.

Patch #2 is a band-aid for the error, but something
like it is needed to stop recursion (which currently
isn't prevented in the repl code at all).

If you want to review and try pushing them I won't
object :-).

Cheers,

	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