Attachment 11194 Details for Bug 11358 – Proposed fix to master

[patch] Proposed fix to master

cancel-child3.patch (text/plain), 4.01 KB, created by Uri Simchoni on 2015-06-24 18:42:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Uri Simchoni

Created: 2015-06-24 18:42:20 UTC

Size: 4.01 KB

patch

obsolete

>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
>

Actions: View

Attachments on bug 11358: 11194 | 11214 | 11215