From 72a08ccd78754ef10509a54454b9dccde472aca7 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni@gmail.com>
Date: Wed, 24 Jun 2015 10:55:06 +0300
Subject: [PATCH v3] winbindd: disconnect child process if request is cancelled
 at main process

When cancelling a request at the main winbindd process, that is currently
being served by a child winbindd process, just freeing all objects related
to the request is not enough, as the next bytes to come through the pipe
from the child process are the response to the cancelled request, and the
object reading those bytes will be the next request. This breaks the protocol.

This change, upon canceling a request that is being served, closes the
connection to the child process, causing the next request to be served
by a new child process (and the detached child to die eventually).

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

Signed-off-by: Uri Simchoni <urisimchoni@gmail.com>
---
 source3/winbindd/winbindd_dual.c | 50 +++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 5bb7cd0..350ec7d 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 *subreq;
 	struct winbindd_child *child;
 	struct winbindd_request *request;
 	struct winbindd_response *response;
@@ -130,6 +131,9 @@ static void wb_child_request_trigger(struct tevent_req *req,
 					    void *private_data);
 static void wb_child_request_done(struct tevent_req *subreq);
 
+static void wb_child_request_cleanup(struct tevent_req *req,
+				     enum tevent_req_state req_state);
+
 struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
 					 struct tevent_context *ev,
 					 struct winbindd_child *child,
@@ -153,6 +157,9 @@ struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
 		tevent_req_oom(req);
 		return tevent_req_post(req, ev);
 	}
+
+	tevent_req_set_cleanup_fn(req, wb_child_request_cleanup);
+
 	return req;
 }
 
@@ -173,6 +180,8 @@ static void wb_child_request_trigger(struct tevent_req *req,
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
+
+	state->subreq = subreq;
 	tevent_req_set_callback(subreq, wb_child_request_done, req);
 	tevent_req_set_endtime(req, state->ev, timeval_current_ofs(300, 0));
 }
@@ -186,15 +195,11 @@ static void wb_child_request_done(struct tevent_req *subreq)
 	int ret, err;
 
 	ret = wb_simple_trans_recv(subreq, state, &state->response, &err);
-	TALLOC_FREE(subreq);
+	/* Freeing the subrequest is deferred until the cleanup function,
+	 * which has to know whether a subrequest exists, and consequently
+	 * decide whether to shut down the pipe to the child process.
+	 */
 	if (ret == -1) {
-		/*
-		 * The basic parent/child communication broke, close
-		 * our socket
-		 */
-		close(state->child->sock);
-		state->child->sock = -1;
-		DLIST_REMOVE(winbindd_children, state->child);
 		tevent_req_error(req, err);
 		return;
 	}
@@ -214,6 +219,35 @@ int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 	return 0;
 }
 
+static void wb_child_request_cleanup(struct tevent_req *req,
+				     enum tevent_req_state req_state)
+{
+	struct wb_child_request_state *state =
+	    tevent_req_data(req, struct wb_child_request_state);
+
+	if (state->subreq == NULL) {
+		/* nothing to cleanup */
+		return;
+	}
+
+	TALLOC_FREE(state->subreq);
+
+	if (req_state == TEVENT_REQ_DONE) {
+		/* transmitted request and got response */
+		return;
+	}
+
+	/*
+	 * Failed to transmit and receive response, or request
+	 * cancelled while being serviced.
+	 * The basic parent/child communication broke, close
+	 * our socket
+	 */
+	close(state->child->sock);
+	state->child->sock = -1;
+	DLIST_REMOVE(winbindd_children, state->child);
+}
+
 static bool winbindd_child_busy(struct winbindd_child *child)
 {
 	return tevent_queue_length(child->queue) > 0;
-- 
1.9.1