From c9d90f001dad00039b9c48105fa0cd83ff58990c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Fri, 9 Feb 2018 10:27:55 +0100
Subject: [PATCH 01/14] winbind: Improve child selection

This improves the situation when a client request blocks a winbind
child. This might be a slow samlogon or lookupnames to a domain that's
far away. With random selection of the child for new request coming in
we could end up with a long queue when other, non-blocked children
could serve those new requests. Choose the shortest queue.

This is an immediate and simple fix. Step two will be to have a
per-domain and not a per-child queue. Right now we're pre-selecting
the check-out queue at Fry's randomly without looking at the queue
length. With this change we're picking the shortest queue. The better
change will be what Fry's really does: One central queue and red/green
lights on the busy/free checkout counters.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13290

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Mon Feb 12 19:51:35 CET 2018 on sn-devel-144

(cherry picked from commit b4384b7f0ecf3b47dd60acaf77636b679e3adc05)
---
 source3/winbindd/winbindd_dual.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index eaa7d9f..59bf389 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -248,33 +248,31 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 	DLIST_REMOVE(winbindd_children, state->child);
 }
 
-static bool winbindd_child_busy(struct winbindd_child *child)
-{
-	return tevent_queue_length(child->queue) > 0;
-}
-
-static struct winbindd_child *find_idle_child(struct winbindd_domain *domain)
+struct winbindd_child *choose_domain_child(struct winbindd_domain *domain)
 {
+	struct winbindd_child *shortest = &domain->children[0];
+	struct winbindd_child *current;
 	int i;
 
 	for (i=0; i<lp_winbind_max_domain_connections(); i++) {
-		if (!winbindd_child_busy(&domain->children[i])) {
-			return &domain->children[i];
-		}
-	}
+		size_t shortest_len, current_len;
 
-	return NULL;
-}
+		current = &domain->children[i];
+		current_len = tevent_queue_length(current->queue);
 
-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain)
-{
-	struct winbindd_child *result;
+		if (current_len == 0) {
+			/* idle child */
+			return current;
+		}
 
-	result = find_idle_child(domain);
-	if (result != NULL) {
-		return result;
+		shortest_len = tevent_queue_length(shortest->queue);
+
+		if (current_len < shortest_len) {
+			shortest = current;
+		}
 	}
-	return &domain->children[rand() % lp_winbind_max_domain_connections()];
+
+	return shortest;
 }
 
 struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain)
-- 
1.9.1


From 956785b7712569624b34cd952b5642b6aa28effb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 16 Feb 2018 15:02:42 +0100
Subject: [PATCH 02/14] winbind: use tevent_queue_wait_send/recv in
 wb_child_request_*()

We need a way to keep the child->queue blocked without relying on
the current 'req' (wb_child_request_state).

The next commit will make use of this.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit d29dda141e08af42c535e8718226f95c45aadab8)
---
 source3/winbindd/winbindd_dual.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 59bf389..1fa9e18 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -119,6 +119,7 @@ static NTSTATUS child_write_response(int sock, struct winbindd_response *wrsp)
 
 struct wb_child_request_state {
 	struct tevent_context *ev;
+	struct tevent_req *queue_subreq;
 	struct tevent_req *subreq;
 	struct winbindd_child *child;
 	struct winbindd_request *request;
@@ -127,8 +128,7 @@ struct wb_child_request_state {
 
 static bool fork_domain_child(struct winbindd_child *child);
 
-static void wb_child_request_trigger(struct tevent_req *req,
-					    void *private_data);
+static void wb_child_request_waited(struct tevent_req *subreq);
 static void wb_child_request_done(struct tevent_req *subreq);
 
 static void wb_child_request_cleanup(struct tevent_req *req,
@@ -141,6 +141,7 @@ struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
 {
 	struct tevent_req *req;
 	struct wb_child_request_state *state;
+	struct tevent_req *subreq;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct wb_child_request_state);
@@ -152,23 +153,36 @@ struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
 	state->child = child;
 	state->request = request;
 
-	if (!tevent_queue_add(child->queue, ev, req,
-			      wb_child_request_trigger, NULL)) {
-		tevent_req_oom(req);
+	subreq = tevent_queue_wait_send(state, ev, child->queue);
+	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
+	tevent_req_set_callback(subreq, wb_child_request_waited, req);
+	state->queue_subreq = subreq;
 
 	tevent_req_set_cleanup_fn(req, wb_child_request_cleanup);
 
 	return req;
 }
 
-static void wb_child_request_trigger(struct tevent_req *req,
-				     void *private_data)
+static void wb_child_request_waited(struct tevent_req *subreq)
 {
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
 	struct wb_child_request_state *state = tevent_req_data(
 		req, struct wb_child_request_state);
-	struct tevent_req *subreq;
+	bool ok;
+
+	ok = tevent_queue_wait_recv(subreq);
+	if (!ok) {
+		tevent_req_oom(req);
+		return;
+	}
+	/*
+	 * We need to keep state->queue_subreq
+	 * in order to block the queue.
+	 */
+	subreq = NULL;
 
 	if ((state->child->sock == -1) && (!fork_domain_child(state->child))) {
 		tevent_req_error(req, errno);
@@ -231,6 +245,7 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 	}
 
 	TALLOC_FREE(state->subreq);
+	TALLOC_FREE(state->queue_subreq);
 
 	if (req_state == TEVENT_REQ_DONE) {
 		/* transmitted request and got response */
-- 
1.9.1


From 3930e31e48a320de89a42899aca6b48648fe3f7b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 16 Feb 2018 15:05:57 +0100
Subject: [PATCH 03/14] winbind: protect a pending wb_child_request against a
 talloc_free()

If the (winbind) client gave up we call TALLOC_FREE(state->mem_ctx)
in remove_client(). This triggers a recursive talloc_free() for all
in flight requests.

In order to maintain the winbindd parent-child protocol, we need
to keep the orphaned wb_simple_trans request until the parent
got the response from the child.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 43af57d8728883c5ddbe169e1483181246fb68a8)
---
 source3/winbindd/winbindd_dual.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 1fa9e18..e3b3b09 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -130,6 +130,7 @@ static bool fork_domain_child(struct winbindd_child *child);
 
 static void wb_child_request_waited(struct tevent_req *subreq);
 static void wb_child_request_done(struct tevent_req *subreq);
+static void wb_child_request_orphaned(struct tevent_req *subreq);
 
 static void wb_child_request_cleanup(struct tevent_req *req,
 				     enum tevent_req_state req_state);
@@ -220,6 +221,12 @@ static void wb_child_request_done(struct tevent_req *subreq)
 	tevent_req_done(req);
 }
 
+static void wb_child_request_orphaned(struct tevent_req *subreq)
+{
+	DBG_WARNING("cleanup orphaned subreq[%p]\n", subreq);
+	TALLOC_FREE(subreq);
+}
+
 int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 			  struct winbindd_response **presponse, int *err)
 {
@@ -244,6 +251,28 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 		return;
 	}
 
+	if (req_state == TEVENT_REQ_RECEIVED) {
+		struct tevent_req *subreq = NULL;
+
+		/*
+		 * Our caller gave up, but we need to keep
+		 * the low level request (wb_simple_trans)
+		 * in order to maintain the parent child protocol.
+		 *
+		 * We also need to keep the child queue blocked
+		 * until we got the response from the child.
+		 */
+
+		subreq = talloc_move(state->child->queue, &state->subreq);
+		talloc_move(subreq, &state->queue_subreq);
+		tevent_req_set_callback(subreq,
+					wb_child_request_orphaned,
+					NULL);
+
+		DBG_WARNING("keep orphaned subreq[%p]\n", subreq);
+		return;
+	}
+
 	TALLOC_FREE(state->subreq);
 	TALLOC_FREE(state->queue_subreq);
 
-- 
1.9.1


From a7c102971981edafab5ed042a51cb7c0883b964e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 16 Feb 2018 16:09:58 +0100
Subject: [PATCH 04/14] winbind: call lp_winbind_enum_{users,groups}() already
 in set{pw,gr}ent()

This way we don't keep winbindd_cli_state->{pw,gr}ent_state arround forever,
if the client forgets an explicit end{pw,gr}ent().

This allows client_is_idle() return true in more cases.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 6548b82b5c1ed30ce14e17e4ba9d4bc24ab49c42)
---
 source3/winbindd/winbindd_getgrent.c | 5 -----
 source3/winbindd/winbindd_getpwent.c | 5 -----
 source3/winbindd/winbindd_setgrent.c | 5 +++++
 source3/winbindd/winbindd_setpwent.c | 5 +++++
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/source3/winbindd/winbindd_getgrent.c b/source3/winbindd/winbindd_getgrent.c
index 21da75b..6a99c6b 100644
--- a/source3/winbindd/winbindd_getgrent.c
+++ b/source3/winbindd/winbindd_getgrent.c
@@ -50,11 +50,6 @@ struct tevent_req *winbindd_getgrent_send(TALLOC_CTX *mem_ctx,
 
 	DEBUG(3, ("[%5lu]: getgrent\n", (unsigned long)cli->pid));
 
-	if (!lp_winbind_enum_groups()) {
-		tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES);
-		return tevent_req_post(req, ev);
-	}
-
 	if (cli->grent_state == NULL) {
 		tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES);
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_getpwent.c b/source3/winbindd/winbindd_getpwent.c
index 3c035ea..d33f5f9 100644
--- a/source3/winbindd/winbindd_getpwent.c
+++ b/source3/winbindd/winbindd_getpwent.c
@@ -49,11 +49,6 @@ struct tevent_req *winbindd_getpwent_send(TALLOC_CTX *mem_ctx,
 
 	DEBUG(3, ("[%5lu]: getpwent\n", (unsigned long)cli->pid));
 
-	if (!lp_winbind_enum_users()) {
-		tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES);
-		return tevent_req_post(req, ev);
-	}
-
 	if (cli->pwent_state == NULL) {
 		tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES);
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_setgrent.c b/source3/winbindd/winbindd_setgrent.c
index 79aa8c3..ab7fa98 100644
--- a/source3/winbindd/winbindd_setgrent.c
+++ b/source3/winbindd/winbindd_setgrent.c
@@ -39,6 +39,11 @@ struct tevent_req *winbindd_setgrent_send(TALLOC_CTX *mem_ctx,
 	}
 	TALLOC_FREE(cli->grent_state);
 
+	if (!lp_winbind_enum_groups()) {
+		tevent_req_done(req);
+		return tevent_req_post(req, ev);
+	}
+
 	cli->grent_state = talloc_zero(cli, struct getgrent_state);
 	if (tevent_req_nomem(cli->grent_state, req)) {
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_setpwent.c b/source3/winbindd/winbindd_setpwent.c
index af28758..4591731 100644
--- a/source3/winbindd/winbindd_setpwent.c
+++ b/source3/winbindd/winbindd_setpwent.c
@@ -39,6 +39,11 @@ struct tevent_req *winbindd_setpwent_send(TALLOC_CTX *mem_ctx,
 	}
 	TALLOC_FREE(cli->pwent_state);
 
+	if (!lp_winbind_enum_users()) {
+		tevent_req_done(req);
+		return tevent_req_post(req, ev);
+	}
+
 	cli->pwent_state = talloc_zero(cli, struct getpwent_state);
 	if (tevent_req_nomem(cli->pwent_state, req)) {
 		return tevent_req_post(req, ev);
-- 
1.9.1


From e893cd0c84d33c6fab078abdce69a9621828ba71 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 16 Feb 2018 16:13:16 +0100
Subject: [PATCH 05/14] winbind: cleanup winbindd_cli_state->grent_state if
 winbindd_getgrent_recv() returns an error

A client may skip the explicit endgrent() if getgrent() fails.

This allows client_is_idle() return true in more cases.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit b7789da8468c3f070727011639d5f74aca76cb59)
---
 source3/winbindd/winbindd_getgrent.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/winbindd/winbindd_getgrent.c b/source3/winbindd/winbindd_getgrent.c
index 6a99c6b..ce1d1e6 100644
--- a/source3/winbindd/winbindd_getgrent.c
+++ b/source3/winbindd/winbindd_getgrent.c
@@ -141,6 +141,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req,
 	int i;
 
 	if (tevent_req_is_nterror(req, &status)) {
+		TALLOC_FREE(state->cli->grent_state);
 		DEBUG(5, ("getgrent failed: %s\n", nt_errstr(status)));
 		return status;
 	}
@@ -151,6 +152,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req,
 
 	memberstrings = talloc_array(talloc_tos(), char *, state->num_groups);
 	if (memberstrings == NULL) {
+		TALLOC_FREE(state->cli->grent_state);
 		return NT_STATUS_NO_MEMORY;
 	}
 
@@ -165,6 +167,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req,
 
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(memberstrings);
+			TALLOC_FREE(state->cli->grent_state);
 			return status;
 		}
 		TALLOC_FREE(state->members[i]);
@@ -180,6 +183,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req,
 	result = talloc_realloc(state, state->groups, char,
 				base_memberofs + total_memberlen);
 	if (result == NULL) {
+		TALLOC_FREE(state->cli->grent_state);
 		return NT_STATUS_NO_MEMORY;
 	}
 	state->groups = (struct winbindd_gr *)result;
-- 
1.9.1


From d9a0b25fd3e298ef751c8a26a1cf12595a376ac6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 16 Feb 2018 16:13:16 +0100
Subject: [PATCH 06/14] winbind: cleanup winbindd_cli_state->pwent_state if
 winbindd_getpwent_recv() returns an error

A client may skip the explicit endpwent() if getgrent() fails.

This allows client_is_idle() return true in more cases.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit b158d4e4c1c3fee0a8884bc5e8f0c5a5ce49687f)
---
 source3/winbindd/winbindd_getpwent.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/winbindd/winbindd_getpwent.c b/source3/winbindd/winbindd_getpwent.c
index d33f5f9..9e5190a 100644
--- a/source3/winbindd/winbindd_getpwent.c
+++ b/source3/winbindd/winbindd_getpwent.c
@@ -124,6 +124,7 @@ NTSTATUS winbindd_getpwent_recv(struct tevent_req *req,
 	NTSTATUS status;
 
 	if (tevent_req_is_nterror(req, &status)) {
+		TALLOC_FREE(state->cli->pwent_state);
 		DEBUG(5, ("getpwent failed: %s\n", nt_errstr(status)));
 		return status;
 	}
-- 
1.9.1


From ce8c4af3a7e5c7a3913fdf722fd94e9652d5eed9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 15 Feb 2018 16:00:33 +0100
Subject: [PATCH 07/14] winbind: avoid using fstrcpy(dcname,...) in
 _dual_init_connection

domain->dcname was converted from fstring to char * by commit
14bae61ba36814ea5eca7c51cf1cc039e9e6803f.

Luckily this was only ever called with an empty string in
state->request->data.init_conn.dcname.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit d73e3d451976e692c6c346f98547d7123f7b9006)
---
 source3/winbindd/winbindd_util.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 370b70c..89b85ea 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -653,7 +653,12 @@ enum winbindd_result winbindd_dual_init_connection(struct winbindd_domain *domai
 		[sizeof(state->request->data.init_conn.dcname)-1]='\0';
 
 	if (strlen(state->request->data.init_conn.dcname) > 0) {
-		fstrcpy(domain->dcname, state->request->data.init_conn.dcname);
+		TALLOC_FREE(domain->dcname);
+		domain->dcname = talloc_strdup(domain,
+				state->request->data.init_conn.dcname);
+		if (domain->dcname == NULL) {
+			return WINBINDD_ERROR;
+		}
 	}
 
 	init_dc_connection(domain, false);
-- 
1.9.1


From 61fa3216f6d8fa7a62ce45e7957be5f41d379e4a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 14 Feb 2018 15:09:51 +0100
Subject: [PATCH 08/14] winbind: use state->{ev,request} in
 wb_domain_request_send()

This will reduce the diff for the following changes.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 4d804f5f3e65df0e2f646d4f88793cab8e2f32d1)
---
 source3/winbindd/winbindd_dual.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index e3b3b09..5045fa0 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -354,11 +354,15 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 		return NULL;
 	}
 
+	state->domain = domain;
+	state->ev = ev;
+	state->request = request;
+
 	state->child = choose_domain_child(domain);
 
 	if (domain->initialized) {
-		subreq = wb_child_request_send(state, ev, state->child,
-					       request);
+		subreq = wb_child_request_send(state, state->ev, state->child,
+					       state->request);
 		if (tevent_req_nomem(subreq, req)) {
 			return tevent_req_post(req, ev);
 		}
@@ -366,10 +370,6 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 		return req;
 	}
 
-	state->domain = domain;
-	state->ev = ev;
-	state->request = request;
-
 	state->init_req = talloc_zero(state, struct winbindd_request);
 	if (tevent_req_nomem(state->init_req, req)) {
 		return tevent_req_post(req, ev);
@@ -382,7 +382,7 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 		state->init_req->data.init_conn.is_primary = domain->primary;
 		fstrcpy(state->init_req->data.init_conn.dcname, "");
 
-		subreq = wb_child_request_send(state, ev, state->child,
+		subreq = wb_child_request_send(state, state->ev, state->child,
 					       state->init_req);
 		if (tevent_req_nomem(subreq, req)) {
 			return tevent_req_post(req, ev);
@@ -403,7 +403,8 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 	state->init_req->cmd = WINBINDD_GETDCNAME;
 	fstrcpy(state->init_req->domain_name, domain->name);
 
-	subreq = wb_child_request_send(state, ev, state->child, request);
+	subreq = wb_child_request_send(state, state->ev, state->child,
+				       state->request);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
-- 
1.9.1


From caf279a97fee935cf62649a6f88f72edbc1650d9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 14 Feb 2018 15:11:50 +0100
Subject: [PATCH 09/14] winbind: improve wb_domain_request_send() to use
 wb_dsgetdcname_send() for a foreign domain

Commit ed3bc614cccec6167c64ac58d78344b6426cd019 got the logic wrong while
trying to implement the logic we had in init_child_connection(),
which was removed by commit d61f3626b79e0523beadff355453145aa7b0195c.

Instead of doing a WINBINDD_GETDCNAME request (which would caused an error
because the implementation was removed in commit
958fdaf5c3ba17969a5110e6b2b08babb9096d7e), we sent the callers request
and interpreted the result as WINBINDD_GETDCNAME response, which
led to an empty dcname variable. As result the domain child
opened a connection to the primary domain in order to lookup
a dc.

If we want to connect the primary domain from the parent via
a domain child of the primary domain.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 1f41193e005df37401a28004f0a95d4d73b98ccd)
---
 source3/winbindd/winbindd_dual.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 5045fa0..06e3c95 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -393,18 +393,18 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 	}
 
 	/*
-	 * Ask our DC for a DC name
+	 * This is *not* the primary domain,
+	 * let's ask our DC about a DC name.
+	 *
+	 * We prefer getting a dns name in dc_unc,
+	 * which is indicated by DS_RETURN_DNS_NAME.
+	 * For NT4 domains we still get the netbios name.
 	 */
-	domain = find_our_domain();
-
-	/* This is *not* the primary domain, let's ask our DC about a DC
-	 * name */
-
-	state->init_req->cmd = WINBINDD_GETDCNAME;
-	fstrcpy(state->init_req->domain_name, domain->name);
-
-	subreq = wb_child_request_send(state, state->ev, state->child,
-				       state->request);
+	subreq = wb_dsgetdcname_send(state, state->ev,
+				     state->domain->name,
+				     NULL, /* domain_guid */
+				     NULL, /* site_name */
+				     DS_RETURN_DNS_NAME); /* flags */
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -418,22 +418,26 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq)
 		subreq, struct tevent_req);
 	struct wb_domain_request_state *state = tevent_req_data(
 		req, struct wb_domain_request_state);
-	struct winbindd_response *response;
-	int ret, err;
+	struct netr_DsRGetDCNameInfo *dcinfo = NULL;
+	NTSTATUS status;
+	const char *dcname = NULL;
 
-	ret = wb_child_request_recv(subreq, talloc_tos(), &response, &err);
+	status = wb_dsgetdcname_recv(subreq, state, &dcinfo);
 	TALLOC_FREE(subreq);
-	if (ret == -1) {
-		tevent_req_error(req, err);
+	if (tevent_req_nterror(req, status)) {
 		return;
 	}
+	dcname = dcinfo->dc_unc;
+	while (dcname != NULL && *dcname == '\\') {
+		dcname++;
+	}
 	state->init_req->cmd = WINBINDD_INIT_CONNECTION;
 	fstrcpy(state->init_req->domain_name, state->domain->name);
 	state->init_req->data.init_conn.is_primary = False;
 	fstrcpy(state->init_req->data.init_conn.dcname,
-		response->data.dc_name);
+		dcname);
 
-	TALLOC_FREE(response);
+	TALLOC_FREE(dcinfo);
 
 	subreq = wb_child_request_send(state, state->ev, state->child,
 				       state->init_req);
-- 
1.9.1


From a41a6048ed3929091b243cc5969a9abff278d6b5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 14 Feb 2018 13:24:54 +0100
Subject: [PATCH 10/14] winbind: add idmap_child_handle() and use it instead of
 child->binding_handle

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit c2d78a0a0a3f9b9ade61cf707f23e59a1a16c61b)
---
 source3/winbindd/wb_sids2xids.c          | 6 +++---
 source3/winbindd/winbindd_allocate_gid.c | 6 +++---
 source3/winbindd/winbindd_allocate_uid.c | 6 +++---
 source3/winbindd/winbindd_idmap.c        | 5 +++++
 source3/winbindd/winbindd_proto.h        | 1 +
 5 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index b8ad300..c687f70 100644
--- a/source3/winbindd/wb_sids2xids.c
+++ b/source3/winbindd/wb_sids2xids.c
@@ -167,7 +167,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq)
 		req, struct wb_sids2xids_state);
 	struct lsa_RefDomainList *domains = NULL;
 	struct lsa_TransNameArray *names = NULL;
-	struct winbindd_child *child;
+	struct dcerpc_binding_handle *child_binding_handle = NULL;
 	NTSTATUS status;
 	int i;
 
@@ -237,7 +237,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq)
 	TALLOC_FREE(names);
 	TALLOC_FREE(domains);
 
-	child = idmap_child();
+	child_binding_handle = idmap_child_handle();
 
 	state->dom_ids = wb_sids2xids_extract_for_domain_index(
 		state, &state->ids, state->dom_index);
@@ -252,7 +252,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq)
 	};
 
 	subreq = dcerpc_wbint_Sids2UnixIDs_send(
-		state, state->ev, child->binding_handle, &state->idmap_dom,
+		state, state->ev, child_binding_handle, &state->idmap_dom,
 		state->dom_ids);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
diff --git a/source3/winbindd/winbindd_allocate_gid.c b/source3/winbindd/winbindd_allocate_gid.c
index a9236bb..85aa136 100644
--- a/source3/winbindd/winbindd_allocate_gid.c
+++ b/source3/winbindd/winbindd_allocate_gid.c
@@ -34,7 +34,7 @@ struct tevent_req *winbindd_allocate_gid_send(TALLOC_CTX *mem_ctx,
 {
 	struct tevent_req *req, *subreq;
 	struct winbindd_allocate_gid_state *state;
-	struct winbindd_child *child;
+	struct dcerpc_binding_handle *child_binding_handle = NULL;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct winbindd_allocate_gid_state);
@@ -44,9 +44,9 @@ struct tevent_req *winbindd_allocate_gid_send(TALLOC_CTX *mem_ctx,
 
 	DEBUG(3, ("allocate_gid\n"));
 
-	child = idmap_child();
+	child_binding_handle = idmap_child_handle();
 
-	subreq = dcerpc_wbint_AllocateGid_send(state, ev, child->binding_handle,
+	subreq = dcerpc_wbint_AllocateGid_send(state, ev, child_binding_handle,
 					       &state->gid);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_allocate_uid.c b/source3/winbindd/winbindd_allocate_uid.c
index 99c0bdac..69ce61c 100644
--- a/source3/winbindd/winbindd_allocate_uid.c
+++ b/source3/winbindd/winbindd_allocate_uid.c
@@ -34,7 +34,7 @@ struct tevent_req *winbindd_allocate_uid_send(TALLOC_CTX *mem_ctx,
 {
 	struct tevent_req *req, *subreq;
 	struct winbindd_allocate_uid_state *state;
-	struct winbindd_child *child;
+	struct dcerpc_binding_handle *child_binding_handle = NULL;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct winbindd_allocate_uid_state);
@@ -44,9 +44,9 @@ struct tevent_req *winbindd_allocate_uid_send(TALLOC_CTX *mem_ctx,
 
 	DEBUG(3, ("allocate_uid\n"));
 
-	child = idmap_child();
+	child_binding_handle = idmap_child_handle();
 
-	subreq = dcerpc_wbint_AllocateUid_send(state, ev, child->binding_handle,
+	subreq = dcerpc_wbint_AllocateUid_send(state, ev, child_binding_handle,
 					       &state->uid);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_idmap.c b/source3/winbindd/winbindd_idmap.c
index 0280260..2ee436b 100644
--- a/source3/winbindd/winbindd_idmap.c
+++ b/source3/winbindd/winbindd_idmap.c
@@ -34,6 +34,11 @@ struct winbindd_child *idmap_child(void)
 	return &static_idmap_child;
 }
 
+struct dcerpc_binding_handle *idmap_child_handle(void)
+{
+	return static_idmap_child.binding_handle;
+}
+
 static const struct winbindd_child_dispatch_table idmap_dispatch_table[] = {
 	{
 		.name		= "PING",
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index e45d9b5..29b0e7e 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -372,6 +372,7 @@ NTSTATUS winbindd_print_groupmembers(struct talloc_dict *members,
 
 void init_idmap_child(void);
 struct winbindd_child *idmap_child(void);
+struct dcerpc_binding_handle *idmap_child_handle(void);
 struct idmap_domain *idmap_find_domain_with_sid(const char *domname,
 						const struct dom_sid *sid);
 const char *idmap_config_const_string(const char *domname, const char *option,
-- 
1.9.1


From 3d2ee84f8bb0066674ceb7addc5783b41420980c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 14 Feb 2018 13:24:54 +0100
Subject: [PATCH 11/14] winbind: add locator_child_handle() and use it instead
 of child->binding_handle

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 44ebaaac8933f5fc16a043b8c15a9449746af47b)
---
 source3/winbindd/wb_dsgetdcname.c       | 8 ++++----
 source3/winbindd/winbindd_dsgetdcname.c | 6 +++---
 source3/winbindd/winbindd_locator.c     | 5 +++++
 source3/winbindd/winbindd_proto.h       | 1 +
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/source3/winbindd/wb_dsgetdcname.c b/source3/winbindd/wb_dsgetdcname.c
index 125e98a..bfc6aa1 100644
--- a/source3/winbindd/wb_dsgetdcname.c
+++ b/source3/winbindd/wb_dsgetdcname.c
@@ -37,7 +37,7 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx,
 {
 	struct tevent_req *req, *subreq;
 	struct wb_dsgetdcname_state *state;
-	struct winbindd_child *child;
+	struct dcerpc_binding_handle *child_binding_handle = NULL;
 	struct GUID guid;
 	struct GUID *guid_ptr = NULL;
 
@@ -72,10 +72,10 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx,
 		/*
 		 * We have to figure out the DC ourselves
 		 */
-		child = locator_child();
+		child_binding_handle = locator_child_handle();
 	} else {
 		struct winbindd_domain *domain = find_our_domain();
-		child = choose_domain_child(domain);
+		child_binding_handle = dom_child_handle(domain);
 	}
 
 	if (domain_guid != NULL) {
@@ -85,7 +85,7 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx,
 	}
 
 	subreq = dcerpc_wbint_DsGetDcName_send(
-		state, ev, child->binding_handle, domain_name, guid_ptr, site_name,
+		state, ev, child_binding_handle, domain_name, guid_ptr, site_name,
 		flags, &state->dcinfo);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
diff --git a/source3/winbindd/winbindd_dsgetdcname.c b/source3/winbindd/winbindd_dsgetdcname.c
index 8eb1de7..fd9270f 100644
--- a/source3/winbindd/winbindd_dsgetdcname.c
+++ b/source3/winbindd/winbindd_dsgetdcname.c
@@ -35,7 +35,7 @@ struct tevent_req *winbindd_dsgetdcname_send(TALLOC_CTX *mem_ctx,
 					     struct winbindd_request *request)
 {
 	struct tevent_req *req, *subreq;
-	struct winbindd_child *child;
+	struct dcerpc_binding_handle *child_binding_handle = NULL;
 	struct winbindd_dsgetdcname_state *state;
 	struct GUID *guid_ptr = NULL;
 	uint32_t ds_flags = 0;
@@ -65,10 +65,10 @@ struct tevent_req *winbindd_dsgetdcname_send(TALLOC_CTX *mem_ctx,
 		guid_ptr = &state->guid;
 	}
 
-	child = locator_child();
+	child_binding_handle = locator_child_handle();
 
 	subreq = dcerpc_wbint_DsGetDcName_send(
-		state, ev, child->binding_handle,
+		state, ev, child_binding_handle,
 		request->data.dsgetdcname.domain_name, guid_ptr,
 		request->data.dsgetdcname.site_name,
 		ds_flags, &state->dc_info);
diff --git a/source3/winbindd/winbindd_locator.c b/source3/winbindd/winbindd_locator.c
index 59e8614..55b6455 100644
--- a/source3/winbindd/winbindd_locator.c
+++ b/source3/winbindd/winbindd_locator.c
@@ -34,6 +34,11 @@ struct winbindd_child *locator_child(void)
 	return &static_locator_child;
 }
 
+struct dcerpc_binding_handle *locator_child_handle(void)
+{
+	return static_locator_child.binding_handle;
+}
+
 static const struct winbindd_child_dispatch_table locator_dispatch_table[] = {
 	{
 		.name		= "PING",
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 29b0e7e..ae48796 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -388,6 +388,7 @@ bool lp_scan_idmap_domains(bool (*fn)(const char *domname,
 
 void init_locator_child(void);
 struct winbindd_child *locator_child(void);
+struct dcerpc_binding_handle *locator_child_handle(void);
 
 /* The following definitions come from winbindd/winbindd_misc.c  */
 
-- 
1.9.1


From 28c94f99baa2eade0b8ed89dedd82f7c6b138a7e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 20 Feb 2018 14:43:38 +0100
Subject: [PATCH 12/14] winbind: make choose_domain_child() static

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 5116aff286bdffe4abc9ddda09cf64ab999fd13e)
---
 source3/winbindd/winbindd_dual.c  | 2 +-
 source3/winbindd/winbindd_proto.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 06e3c95..6d249fe 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -292,7 +292,7 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 	DLIST_REMOVE(winbindd_children, state->child);
 }
 
-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain)
+static struct winbindd_child *choose_domain_child(struct winbindd_domain *domain)
 {
 	struct winbindd_child *shortest = &domain->children[0];
 	struct winbindd_child *current;
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index ae48796..a300a53 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -295,7 +295,6 @@ void setup_domain_child(struct winbindd_domain *domain);
 /* The following definitions come from winbindd/winbindd_dual.c  */
 
 struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain);
-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain);
 
 struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
 					 struct tevent_context *ev,
-- 
1.9.1


From e13ef0bd9c8f6436b337a5b89fec4e79ee8086fc Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Tue, 13 Feb 2018 16:04:44 +0100
Subject: [PATCH 13/14] winbind: Maintain a binding handle per domain and
 always go via wb_domain_request_send()

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit b518cb0597d269002105644302c58ca8f9f0f717)
---
 source3/winbindd/winbindd.h          |  2 ++
 source3/winbindd/winbindd_dual.c     | 11 +++----
 source3/winbindd/winbindd_dual_ndr.c | 61 +++++++++++++++++++++++++++++++-----
 source3/winbindd/winbindd_util.c     |  6 ++++
 4 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 1c7e4e9..25e4812 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -185,6 +185,8 @@ struct winbindd_domain {
 
 	struct winbindd_child *children;
 
+	struct dcerpc_binding_handle *binding_handle;
+
 	/* Callback we use to try put us back online. */
 
 	uint32_t check_online_timeout;
diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 6d249fe..89823b7 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -321,10 +321,7 @@ static struct winbindd_child *choose_domain_child(struct winbindd_domain *domain
 
 struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain)
 {
-	struct winbindd_child *child;
-
-	child = choose_domain_child(domain);
-	return child->binding_handle;
+	return domain->binding_handle;
 }
 
 struct wb_domain_request_state {
@@ -608,8 +605,10 @@ void setup_child(struct winbindd_domain *domain, struct winbindd_child *child,
 	child->table = table;
 	child->queue = tevent_queue_create(NULL, "winbind_child");
 	SMB_ASSERT(child->queue != NULL);
-	child->binding_handle = wbint_binding_handle(NULL, domain, child);
-	SMB_ASSERT(child->binding_handle != NULL);
+	if (domain == NULL) {
+		child->binding_handle = wbint_binding_handle(NULL, NULL, child);
+		SMB_ASSERT(child->binding_handle != NULL);
+	}
 }
 
 void winbind_child_died(pid_t pid)
diff --git a/source3/winbindd/winbindd_dual_ndr.c b/source3/winbindd/winbindd_dual_ndr.c
index 250d9d3..fca1996 100644
--- a/source3/winbindd/winbindd_dual_ndr.c
+++ b/source3/winbindd/winbindd_dual_ndr.c
@@ -42,7 +42,7 @@ static bool wbint_bh_is_connected(struct dcerpc_binding_handle *h)
 	struct wbint_bh_state *hs = dcerpc_binding_handle_data(h,
 				     struct wbint_bh_state);
 
-	if (!hs->child) {
+	if ((hs->domain == NULL) && (hs->child == NULL)) {
 		return false;
 	}
 
@@ -65,7 +65,8 @@ struct wbint_bh_raw_call_state {
 	DATA_BLOB out_data;
 };
 
-static void wbint_bh_raw_call_done(struct tevent_req *subreq);
+static void wbint_bh_raw_call_child_done(struct tevent_req *subreq);
+static void wbint_bh_raw_call_domain_done(struct tevent_req *subreq);
 
 static struct tevent_req *wbint_bh_raw_call_send(TALLOC_CTX *mem_ctx,
 						  struct tevent_context *ev,
@@ -112,17 +113,28 @@ static struct tevent_req *wbint_bh_raw_call_send(TALLOC_CTX *mem_ctx,
 	state->request.extra_data.data = (char *)state->in_data.data;
 	state->request.extra_len = state->in_data.length;
 
-	subreq = wb_child_request_send(state, ev, hs->child,
-				       &state->request);
+	if (hs->child != NULL) {
+		subreq = wb_child_request_send(state, ev, hs->child,
+					       &state->request);
+		if (tevent_req_nomem(subreq, req)) {
+			return tevent_req_post(req, ev);
+		}
+		tevent_req_set_callback(
+			subreq, wbint_bh_raw_call_child_done, req);
+		return req;
+	}
+
+	subreq = wb_domain_request_send(state, ev, hs->domain,
+					&state->request);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
-	tevent_req_set_callback(subreq, wbint_bh_raw_call_done, req);
+	tevent_req_set_callback(subreq, wbint_bh_raw_call_domain_done, req);
 
 	return req;
 }
 
-static void wbint_bh_raw_call_done(struct tevent_req *subreq)
+static void wbint_bh_raw_call_child_done(struct tevent_req *subreq)
 {
 	struct tevent_req *req =
 		tevent_req_callback_data(subreq,
@@ -156,6 +168,40 @@ static void wbint_bh_raw_call_done(struct tevent_req *subreq)
 	tevent_req_done(req);
 }
 
+static void wbint_bh_raw_call_domain_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct wbint_bh_raw_call_state *state =
+		tevent_req_data(req,
+		struct wbint_bh_raw_call_state);
+	int ret, err;
+
+	ret = wb_domain_request_recv(subreq, state, &state->response, &err);
+	TALLOC_FREE(subreq);
+	if (ret == -1) {
+		NTSTATUS status = map_nt_error_from_unix(err);
+		tevent_req_nterror(req, status);
+		return;
+	}
+
+	state->out_data = data_blob_talloc(state,
+		state->response->extra_data.data,
+		state->response->length - sizeof(struct winbindd_response));
+	if (state->response->extra_data.data && !state->out_data.data) {
+		tevent_req_oom(req);
+		return;
+	}
+
+	if (state->domain != NULL) {
+		wcache_store_ndr(state->domain, state->opnum,
+				 &state->in_data, &state->out_data);
+	}
+
+	tevent_req_done(req);
+}
+
 static NTSTATUS wbint_bh_raw_call_recv(struct tevent_req *req,
 					TALLOC_CTX *mem_ctx,
 					uint8_t **out_data,
@@ -207,9 +253,8 @@ static struct tevent_req *wbint_bh_disconnect_send(TALLOC_CTX *mem_ctx,
 
 	/*
 	 * TODO: do a real async disconnect ...
-	 *
-	 * For now the caller needs to free rpc_cli
 	 */
+	hs->domain = NULL;
 	hs->child = NULL;
 
 	tevent_req_done(req);
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 89b85ea..a7e769e 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -222,6 +222,12 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc)
 		return NULL;
 	}
 
+	domain->binding_handle = wbint_binding_handle(domain, domain, NULL);
+	if (domain->binding_handle == NULL) {
+		TALLOC_FREE(domain);
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	domain->name = talloc_strdup(domain, domain_name);
 	if (domain->name == NULL) {
 		TALLOC_FREE(domain);
-- 
1.9.1


From b71bcaf7abae4ebbb8b042093f77279a6bd310d3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 14 Feb 2018 15:04:01 +0100
Subject: [PATCH 14/14] winbind: Use one queue for all domain children

If we have multiple domain children, it's important
that the first idle child takes over the next waiting request.

Before we had the problem that a request could get stuck in the
queue of a busy child, while later requests could get served fine by
other children.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Fri Feb 23 09:04:23 CET 2018 on sn-devel-144

(cherry picked from commit 7f2d45a6c2a88dd8833fc66d314ec21507dd52c3)
---
 source3/winbindd/winbindd.h      |   1 +
 source3/winbindd/winbindd_dual.c | 127 ++++++++++++++++++++++++++++++++++++---
 source3/winbindd/winbindd_util.c |   6 ++
 3 files changed, 125 insertions(+), 9 deletions(-)

diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 25e4812..ebb9aa1 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -185,6 +185,7 @@ struct winbindd_domain {
 
 	struct winbindd_child *children;
 
+	struct tevent_queue *queue;
 	struct dcerpc_binding_handle *binding_handle;
 
 	/* Callback we use to try put us back online. */
diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 89823b7..512ecd0 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -223,8 +223,21 @@ static void wb_child_request_done(struct tevent_req *subreq)
 
 static void wb_child_request_orphaned(struct tevent_req *subreq)
 {
+	struct winbindd_child *child =
+		(struct winbindd_child *)tevent_req_callback_data_void(subreq);
+
 	DBG_WARNING("cleanup orphaned subreq[%p]\n", subreq);
 	TALLOC_FREE(subreq);
+
+	if (child->domain != NULL) {
+		/*
+		 * If the child is attached to a domain,
+		 * we need to make sure the domain queue
+		 * can move forward, after the orphaned
+		 * request is done.
+		 */
+		tevent_queue_start(child->domain->queue);
+	}
 }
 
 int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
@@ -267,7 +280,7 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 		talloc_move(subreq, &state->queue_subreq);
 		tevent_req_set_callback(subreq,
 					wb_child_request_orphaned,
-					NULL);
+					state->child);
 
 		DBG_WARNING("keep orphaned subreq[%p]\n", subreq);
 		return;
@@ -276,6 +289,16 @@ static void wb_child_request_cleanup(struct tevent_req *req,
 	TALLOC_FREE(state->subreq);
 	TALLOC_FREE(state->queue_subreq);
 
+	if (state->child->domain != NULL) {
+		/*
+		 * If the child is attached to a domain,
+		 * we need to make sure the domain queue
+		 * can move forward, after the request
+		 * is done.
+		 */
+		tevent_queue_start(state->child->domain->queue);
+	}
+
 	if (req_state == TEVENT_REQ_DONE) {
 		/* transmitted request and got response */
 		return;
@@ -326,13 +349,35 @@ struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain)
 
 struct wb_domain_request_state {
 	struct tevent_context *ev;
+	struct tevent_queue_entry *queue_entry;
 	struct winbindd_domain *domain;
 	struct winbindd_child *child;
 	struct winbindd_request *request;
 	struct winbindd_request *init_req;
 	struct winbindd_response *response;
+	struct tevent_req *pending_subreq;
 };
 
+static void wb_domain_request_cleanup(struct tevent_req *req,
+				      enum tevent_req_state req_state)
+{
+	struct wb_domain_request_state *state = tevent_req_data(
+		req, struct wb_domain_request_state);
+
+	/*
+	 * If we're completely done or got a failure.
+	 * we should remove ourself from the domain queue,
+	 * after removing the child subreq from the child queue
+	 * and give the next one in the queue the chance
+	 * to check for an idle child.
+	 */
+	TALLOC_FREE(state->pending_subreq);
+	TALLOC_FREE(state->queue_entry);
+	tevent_queue_start(state->domain->queue);
+}
+
+static void wb_domain_request_trigger(struct tevent_req *req,
+				      void *private_data);
 static void wb_domain_request_gotdc(struct tevent_req *subreq);
 static void wb_domain_request_initialized(struct tevent_req *subreq);
 static void wb_domain_request_done(struct tevent_req *subreq);
@@ -342,7 +387,7 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 					  struct winbindd_domain *domain,
 					  struct winbindd_request *request)
 {
-	struct tevent_req *req, *subreq;
+	struct tevent_req *req;
 	struct wb_domain_request_state *state;
 
 	req = tevent_req_create(mem_ctx, &state,
@@ -355,21 +400,66 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 	state->ev = ev;
 	state->request = request;
 
+	tevent_req_set_cleanup_fn(req, wb_domain_request_cleanup);
+
+	state->queue_entry = tevent_queue_add_entry(
+			domain->queue, state->ev, req,
+			wb_domain_request_trigger, NULL);
+	if (tevent_req_nomem(state->queue_entry, req)) {
+		return tevent_req_post(req, ev);
+	}
+
+	return req;
+}
+
+static void wb_domain_request_trigger(struct tevent_req *req,
+				      void *private_data)
+{
+	struct wb_domain_request_state *state = tevent_req_data(
+		req, struct wb_domain_request_state);
+	struct winbindd_domain *domain = state->domain;
+	struct tevent_req *subreq = NULL;
+	size_t shortest_queue_length;
+
 	state->child = choose_domain_child(domain);
+	shortest_queue_length = tevent_queue_length(state->child->queue);
+	if (shortest_queue_length > 0) {
+		/*
+		 * All children are busy, we need to stop
+		 * the queue and untrigger our own queue
+		 * entry. Once a pending request
+		 * is done it calls tevent_queue_start
+		 * and we get retriggered.
+		 */
+		state->child = NULL;
+		tevent_queue_stop(state->domain->queue);
+		tevent_queue_entry_untrigger(state->queue_entry);
+		return;
+	}
 
 	if (domain->initialized) {
 		subreq = wb_child_request_send(state, state->ev, state->child,
 					       state->request);
 		if (tevent_req_nomem(subreq, req)) {
-			return tevent_req_post(req, ev);
+			return;
 		}
 		tevent_req_set_callback(subreq, wb_domain_request_done, req);
-		return req;
+		state->pending_subreq = subreq;
+
+		/*
+		 * Once the domain is initialized and
+		 * once we placed our real request into the child queue,
+		 * we can remove ourself from the domain queue
+		 * and give the next one in the queue the chance
+		 * to check for an idle child.
+		 */
+		TALLOC_FREE(state->queue_entry);
+		return;
 	}
 
 	state->init_req = talloc_zero(state, struct winbindd_request);
 	if (tevent_req_nomem(state->init_req, req)) {
-		return tevent_req_post(req, ev);
+		return;
 	}
 
 	if (IS_DC || domain->primary || domain->internal) {
@@ -382,11 +472,12 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 		subreq = wb_child_request_send(state, state->ev, state->child,
 					       state->init_req);
 		if (tevent_req_nomem(subreq, req)) {
-			return tevent_req_post(req, ev);
+			return;
 		}
 		tevent_req_set_callback(subreq, wb_domain_request_initialized,
 					req);
-		return req;
+		state->pending_subreq = subreq;
+		return;
 	}
 
 	/*
@@ -403,10 +494,11 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx,
 				     NULL, /* site_name */
 				     DS_RETURN_DNS_NAME); /* flags */
 	if (tevent_req_nomem(subreq, req)) {
-		return tevent_req_post(req, ev);
+		return;
 	}
 	tevent_req_set_callback(subreq, wb_domain_request_gotdc, req);
-	return req;
+	state->pending_subreq = subreq;
+	return;
 }
 
 static void wb_domain_request_gotdc(struct tevent_req *subreq)
@@ -419,6 +511,8 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq)
 	NTSTATUS status;
 	const char *dcname = NULL;
 
+	state->pending_subreq = NULL;
+
 	status = wb_dsgetdcname_recv(subreq, state, &dcinfo);
 	TALLOC_FREE(subreq);
 	if (tevent_req_nterror(req, status)) {
@@ -442,6 +536,7 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq)
 		return;
 	}
 	tevent_req_set_callback(subreq, wb_domain_request_initialized, req);
+	state->pending_subreq = subreq;
 }
 
 static void wb_domain_request_initialized(struct tevent_req *subreq)
@@ -453,6 +548,8 @@ static void wb_domain_request_initialized(struct tevent_req *subreq)
 	struct winbindd_response *response;
 	int ret, err;
 
+	state->pending_subreq = NULL;
+
 	ret = wb_child_request_recv(subreq, talloc_tos(), &response, &err);
 	TALLOC_FREE(subreq);
 	if (ret == -1) {
@@ -500,6 +597,16 @@ static void wb_domain_request_initialized(struct tevent_req *subreq)
 		return;
 	}
 	tevent_req_set_callback(subreq, wb_domain_request_done, req);
+	state->pending_subreq = subreq;
+
+	/*
+	 * Once the domain is initialized and
+	 * once we placed our real request into the child queue,
+	 * we can remove ourself from the domain queue
+	 * and give the next one in the queue the chance
+	 * to check for an idle child.
+	 */
+	TALLOC_FREE(state->queue_entry);
 }
 
 static void wb_domain_request_done(struct tevent_req *subreq)
@@ -510,6 +617,8 @@ static void wb_domain_request_done(struct tevent_req *subreq)
 		req, struct wb_domain_request_state);
 	int ret, err;
 
+	state->pending_subreq = NULL;
+
 	ret = wb_child_request_recv(subreq, talloc_tos(), &state->response,
 				    &err);
 	TALLOC_FREE(subreq);
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index a7e769e..cc92112 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -222,6 +222,12 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc)
 		return NULL;
 	}
 
+	domain->queue = tevent_queue_create(domain, "winbind_domain");
+	if (domain->queue == NULL) {
+		TALLOC_FREE(domain);
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	domain->binding_handle = wbint_binding_handle(domain, domain, NULL);
 	if (domain->binding_handle == NULL) {
 		TALLOC_FREE(domain);
-- 
1.9.1