wbclient: fix wbcLookupName with UPN

Stefan Metzmacher metze at samba.org
Thu Feb 22 13:12:47 UTC 2018


Hi Isaac,

> The attached patch fixes the case where the UPN differs from
> user at domain (both components can be different).
> It works fine even when empty domain is specified.
> 
> Review appreciated.
> 
> btw, any thoughts on this pull request?
> https://github.com/samba-team/samba/pull/102

I think you're patch will only work on a domain member server,
as there the find_lookup_domain_from_name() within wb_lookupname_send()
will always return our primary domain.

But on a DC an empty domain string doesn't allow to find the domain.

Maybe we need to add an additional parameter to wb_lookupname_send()
to indicate the namespace.

struct tevent_req *wb_lookupname_send(TALLOC_CTX *mem_ctx,
                                      struct tevent_context *ev,
				      const char *namespace,
                                      const char *dom_name,
				      const char *name,
                                      uint32_t flags)


The namespace is either the domain name or the part after the @,
it would be passed to find_lookup_domain_from_name()

The attached patch might be a start for this, but I think
should unify the logic in parse_domain_user() and also pass a namespace
to it.

Then we also need some simple tests for it.

metze
-------------- next part --------------
From a40ed450024ac9187af5d941e4e9901970f09a52 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 16:47:57 +0100
Subject: [PATCH 01/16] tevent: improve documentation of
 tevent_queue_add_optimize_empty()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/tevent.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h
index 7284a85..c17d4e1 100644
--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -1611,6 +1611,9 @@ struct tevent_queue_entry *tevent_queue_add_entry(
  * already called tevent_req_notify_callback(), tevent_req_error(),
  * tevent_req_done() or a similar function.
  *
+ * The trigger function has no chance to see the returned
+ * queue_entry in the optimized case.
+ *
  * The request can be removed from the queue by calling talloc_free()
  * (or a similar function) on the returned queue entry.
  *
-- 
1.9.1


From e3834bead52aa9cc029f66b5dded4de187acbced Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 15 Feb 2018 14:47:25 +0100
Subject: [PATCH 02/16] tevent: add tevent_queue_entry_untrigger()

Pair-Programmed-With: Volker Lendecke <vl at samba.org>

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/tevent.h       | 22 ++++++++++++++++++++++
 lib/tevent/tevent_queue.c | 13 +++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h
index c17d4e1..7bb9c61 100644
--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -1644,6 +1644,28 @@ struct tevent_queue_entry *tevent_queue_add_optimize_empty(
 					void *private_data);
 
 /**
+ * @brief Untrigger an already triggered queue entry.
+ *
+ * If a trigger function detects that it needs to remain
+ * in the queue, it needs to call tevent_queue_stop()
+ * followed by tevent_queue_entry_untrigger().
+ *
+ * @note In order to call tevent_queue_entry_untrigger()
+ * the queue must be already stopped and the given queue_entry
+ * must be the first one in the queue! Otherwise it calls abort().
+ *
+ * @note You can't use this together with tevent_queue_add_optimize_empty()
+ * because the trigger function don't have access to the quene entry
+ * in the case of an empty queue.
+ *
+ * @param[in]  queue_entry The queue entry to rearm.
+ *
+ * @see tevent_queue_add_entry()
+ * @see tevent_queue_stop()
+ */
+void tevent_queue_entry_untrigger(struct tevent_queue_entry *entry);
+
+/**
  * @brief Start a tevent queue.
  *
  * The queue is started by default.
diff --git a/lib/tevent/tevent_queue.c b/lib/tevent/tevent_queue.c
index 5516c6c..9c3973b 100644
--- a/lib/tevent/tevent_queue.c
+++ b/lib/tevent/tevent_queue.c
@@ -266,6 +266,19 @@ struct tevent_queue_entry *tevent_queue_add_optimize_empty(
 					 trigger, private_data, true);
 }
 
+void tevent_queue_entry_untrigger(struct tevent_queue_entry *entry)
+{
+	if (entry->queue->running) {
+		abort();
+	}
+
+	if (entry->queue->list != entry) {
+		abort();
+	}
+
+	entry->triggered = false;
+}
+
 void tevent_queue_start(struct tevent_queue *queue)
 {
 	if (queue->running) {
-- 
1.9.1


From df62e5b3b536c7c772cb1ab597dd4ce43d1feef6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 20 Feb 2018 13:54:49 +0100
Subject: [PATCH 03/16] tevent: version 0.9.36

* improve documentation of tevent_queue_add_optimize_empty()
* add tevent_queue_entry_untrigger()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/ABI/tevent-0.9.36.sigs | 100 ++++++++++++++++++++++++++++++++++++++
 lib/tevent/wscript                |   2 +-
 2 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 lib/tevent/ABI/tevent-0.9.36.sigs

diff --git a/lib/tevent/ABI/tevent-0.9.36.sigs b/lib/tevent/ABI/tevent-0.9.36.sigs
new file mode 100644
index 0000000..8a579c8
--- /dev/null
+++ b/lib/tevent/ABI/tevent-0.9.36.sigs
@@ -0,0 +1,100 @@
+_tevent_add_fd: struct tevent_fd *(struct tevent_context *, TALLOC_CTX *, int, uint16_t, tevent_fd_handler_t, void *, const char *, const char *)
+_tevent_add_signal: struct tevent_signal *(struct tevent_context *, TALLOC_CTX *, int, int, tevent_signal_handler_t, void *, const char *, const char *)
+_tevent_add_timer: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+_tevent_create_immediate: struct tevent_immediate *(TALLOC_CTX *, const char *)
+_tevent_loop_once: int (struct tevent_context *, const char *)
+_tevent_loop_until: int (struct tevent_context *, bool (*)(void *), void *, const char *)
+_tevent_loop_wait: int (struct tevent_context *, const char *)
+_tevent_queue_create: struct tevent_queue *(TALLOC_CTX *, const char *, const char *)
+_tevent_req_callback_data: void *(struct tevent_req *)
+_tevent_req_cancel: bool (struct tevent_req *, const char *)
+_tevent_req_create: struct tevent_req *(TALLOC_CTX *, void *, size_t, const char *, const char *)
+_tevent_req_data: void *(struct tevent_req *)
+_tevent_req_done: void (struct tevent_req *, const char *)
+_tevent_req_error: bool (struct tevent_req *, uint64_t, const char *)
+_tevent_req_nomem: bool (const void *, struct tevent_req *, const char *)
+_tevent_req_notify_callback: void (struct tevent_req *, const char *)
+_tevent_req_oom: void (struct tevent_req *, const char *)
+_tevent_schedule_immediate: void (struct tevent_immediate *, struct tevent_context *, tevent_immediate_handler_t, void *, const char *, const char *)
+_tevent_threaded_schedule_immediate: void (struct tevent_threaded_context *, struct tevent_immediate *, tevent_immediate_handler_t, void *, const char *, const char *)
+tevent_backend_list: const char **(TALLOC_CTX *)
+tevent_cleanup_pending_signal_handlers: void (struct tevent_signal *)
+tevent_common_add_fd: struct tevent_fd *(struct tevent_context *, TALLOC_CTX *, int, uint16_t, tevent_fd_handler_t, void *, const char *, const char *)
+tevent_common_add_signal: struct tevent_signal *(struct tevent_context *, TALLOC_CTX *, int, int, tevent_signal_handler_t, void *, const char *, const char *)
+tevent_common_add_timer: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+tevent_common_add_timer_v2: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+tevent_common_check_signal: int (struct tevent_context *)
+tevent_common_context_destructor: int (struct tevent_context *)
+tevent_common_fd_destructor: int (struct tevent_fd *)
+tevent_common_fd_get_flags: uint16_t (struct tevent_fd *)
+tevent_common_fd_set_close_fn: void (struct tevent_fd *, tevent_fd_close_fn_t)
+tevent_common_fd_set_flags: void (struct tevent_fd *, uint16_t)
+tevent_common_have_events: bool (struct tevent_context *)
+tevent_common_loop_immediate: bool (struct tevent_context *)
+tevent_common_loop_timer_delay: struct timeval (struct tevent_context *)
+tevent_common_loop_wait: int (struct tevent_context *, const char *)
+tevent_common_schedule_immediate: void (struct tevent_immediate *, struct tevent_context *, tevent_immediate_handler_t, void *, const char *, const char *)
+tevent_common_threaded_activate_immediate: void (struct tevent_context *)
+tevent_common_wakeup: int (struct tevent_context *)
+tevent_common_wakeup_fd: int (int)
+tevent_common_wakeup_init: int (struct tevent_context *)
+tevent_context_init: struct tevent_context *(TALLOC_CTX *)
+tevent_context_init_byname: struct tevent_context *(TALLOC_CTX *, const char *)
+tevent_context_init_ops: struct tevent_context *(TALLOC_CTX *, const struct tevent_ops *, void *)
+tevent_debug: void (struct tevent_context *, enum tevent_debug_level, const char *, ...)
+tevent_fd_get_flags: uint16_t (struct tevent_fd *)
+tevent_fd_set_auto_close: void (struct tevent_fd *)
+tevent_fd_set_close_fn: void (struct tevent_fd *, tevent_fd_close_fn_t)
+tevent_fd_set_flags: void (struct tevent_fd *, uint16_t)
+tevent_get_trace_callback: void (struct tevent_context *, tevent_trace_callback_t *, void *)
+tevent_loop_allow_nesting: void (struct tevent_context *)
+tevent_loop_set_nesting_hook: void (struct tevent_context *, tevent_nesting_hook, void *)
+tevent_num_signals: size_t (void)
+tevent_queue_add: bool (struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_add_entry: struct tevent_queue_entry *(struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_add_optimize_empty: struct tevent_queue_entry *(struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_entry_untrigger: void (struct tevent_queue_entry *)
+tevent_queue_length: size_t (struct tevent_queue *)
+tevent_queue_running: bool (struct tevent_queue *)
+tevent_queue_start: void (struct tevent_queue *)
+tevent_queue_stop: void (struct tevent_queue *)
+tevent_queue_wait_recv: bool (struct tevent_req *)
+tevent_queue_wait_send: struct tevent_req *(TALLOC_CTX *, struct tevent_context *, struct tevent_queue *)
+tevent_re_initialise: int (struct tevent_context *)
+tevent_register_backend: bool (const char *, const struct tevent_ops *)
+tevent_req_default_print: char *(struct tevent_req *, TALLOC_CTX *)
+tevent_req_defer_callback: void (struct tevent_req *, struct tevent_context *)
+tevent_req_is_error: bool (struct tevent_req *, enum tevent_req_state *, uint64_t *)
+tevent_req_is_in_progress: bool (struct tevent_req *)
+tevent_req_poll: bool (struct tevent_req *, struct tevent_context *)
+tevent_req_post: struct tevent_req *(struct tevent_req *, struct tevent_context *)
+tevent_req_print: char *(TALLOC_CTX *, struct tevent_req *)
+tevent_req_received: void (struct tevent_req *)
+tevent_req_reset_endtime: void (struct tevent_req *)
+tevent_req_set_callback: void (struct tevent_req *, tevent_req_fn, void *)
+tevent_req_set_cancel_fn: void (struct tevent_req *, tevent_req_cancel_fn)
+tevent_req_set_cleanup_fn: void (struct tevent_req *, tevent_req_cleanup_fn)
+tevent_req_set_endtime: bool (struct tevent_req *, struct tevent_context *, struct timeval)
+tevent_req_set_print_fn: void (struct tevent_req *, tevent_req_print_fn)
+tevent_sa_info_queue_count: size_t (void)
+tevent_set_abort_fn: void (void (*)(const char *))
+tevent_set_debug: int (struct tevent_context *, void (*)(void *, enum tevent_debug_level, const char *, va_list), void *)
+tevent_set_debug_stderr: int (struct tevent_context *)
+tevent_set_default_backend: void (const char *)
+tevent_set_trace_callback: void (struct tevent_context *, tevent_trace_callback_t, void *)
+tevent_signal_support: bool (struct tevent_context *)
+tevent_thread_proxy_create: struct tevent_thread_proxy *(struct tevent_context *)
+tevent_thread_proxy_schedule: void (struct tevent_thread_proxy *, struct tevent_immediate **, tevent_immediate_handler_t, void *)
+tevent_threaded_context_create: struct tevent_threaded_context *(TALLOC_CTX *, struct tevent_context *)
+tevent_timeval_add: struct timeval (const struct timeval *, uint32_t, uint32_t)
+tevent_timeval_compare: int (const struct timeval *, const struct timeval *)
+tevent_timeval_current: struct timeval (void)
+tevent_timeval_current_ofs: struct timeval (uint32_t, uint32_t)
+tevent_timeval_is_zero: bool (const struct timeval *)
+tevent_timeval_set: struct timeval (uint32_t, uint32_t)
+tevent_timeval_until: struct timeval (const struct timeval *, const struct timeval *)
+tevent_timeval_zero: struct timeval (void)
+tevent_trace_point_callback: void (struct tevent_context *, enum tevent_trace_point)
+tevent_update_timer: void (struct tevent_timer *, struct timeval)
+tevent_wakeup_recv: bool (struct tevent_req *)
+tevent_wakeup_send: struct tevent_req *(TALLOC_CTX *, struct tevent_context *, struct timeval)
diff --git a/lib/tevent/wscript b/lib/tevent/wscript
index 2c67f1f..94d190f 100644
--- a/lib/tevent/wscript
+++ b/lib/tevent/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'tevent'
-VERSION = '0.9.35'
+VERSION = '0.9.36'
 
 blddir = 'bin'
 
-- 
1.9.1


From 8454a867d400f5627ab0ad22214e37c8d37da37a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 15:02:42 +0100
Subject: [PATCH 04/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 7fb5aa8..fa5b5b0 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 3eadd55cfb60f6f96f7203d47614b5f32e8047c8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 15:05:57 +0100
Subject: [PATCH 05/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 fa5b5b0..d0a40a8b 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 91687d435ce5187f4fa0a764d732d95154123494 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 16:09:58 +0100
Subject: [PATCH 06/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 2c8cbac..1056555 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 e08fca8e7a71deb329b7f4829e2bc3e76052f50f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 16:13:16 +0100
Subject: [PATCH 07/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 1056555..15a35f7 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 04b64d9bfa8a362b845070ce1b774811d091bdde Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Feb 2018 16:13:16 +0100
Subject: [PATCH 08/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 1ed80fee18aab95bbd010a49858554e37da54851 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 15 Feb 2018 16:00:33 +0100
Subject: [PATCH 09/16] 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 at samba.org>
---
 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 6292cce..9950c66 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -781,7 +781,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 3ef3654c812c3e537e913d778565e70684e23301 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Feb 2018 15:09:51 +0100
Subject: [PATCH 10/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 d0a40a8b..cd67aec 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 9f4bc5a4636cc3b0f3ea88848eb59d88bd258484 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Feb 2018 15:11:50 +0100
Subject: [PATCH 11/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 cd67aec..1dd8b6f 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 c53fb8eaef545d57fcf1b303547338c9bd98073e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Feb 2018 13:24:54 +0100
Subject: [PATCH 12/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 d09176d..66d4a19 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -351,6 +351,7 @@ NTSTATUS winbindd_print_groupmembers(struct db_context *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 ac00c908daccefee3a7a34cd4dcd1663595fff61 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Feb 2018 13:24:54 +0100
Subject: [PATCH 13/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 8bd7419..2f450c7 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 66d4a19..5d31490 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -367,6 +367,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 9ff244bfa1e576acf6125610a6ae8ec5022a5da0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 20 Feb 2018 14:43:38 +0100
Subject: [PATCH 14/16] winbind: make choose_domain_child() static

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 1dd8b6f..21fe3c2 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 5d31490..704a8dd 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -272,7 +272,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 f76f0203ff05d24d582e1cf750dd6a1bb01b0e80 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 13 Feb 2018 16:04:44 +0100
Subject: [PATCH 15/16] winbind: Maintain a binding handle per domain and
 always go via wb_domain_request_send()

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

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Volker Lendecke <vl at samba.org>
---
 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 3e4b256..8a44f37 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -184,6 +184,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 21fe3c2..a30ac36 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 00c7df1..25e7445 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,
@@ -114,17 +115,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,
@@ -158,6 +170,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,
@@ -209,9 +255,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 9950c66..78f526c 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -228,6 +228,12 @@ static NTSTATUS add_trusted_domain(const char *domain_name,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+	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 636d0b0f41f8999ac95134f3fc09289122c488f3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Feb 2018 15:04:01 +0100
Subject: [PATCH 16/16] 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 at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 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 8a44f37..081722f 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -184,6 +184,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 a30ac36..874d556 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 78f526c..73e6b76 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -228,6 +228,12 @@ static NTSTATUS add_trusted_domain(const char *domain_name,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+	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

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


More information about the samba-technical mailing list