Attachment 11214 Details for Bug 11358 – Patches for v4-2-test

[patch] Patches for v4-2-test

tmp42.diff.txt (text/plain), 4.30 KB, created by Stefan Metzmacher on 2015-06-30 08:21:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2015-06-30 08:21:15 UTC

Size: 4.30 KB

patch

obsolete

>From 0a82aa446556f3674e39bfb8e0188c45b8c2624a Mon Sep 17 00:00:00 2001
>From: Uri Simchoni <urisimchoni@gmail.com>
>Date: Wed, 24 Jun 2015 10:55:06 +0300
>Subject: [PATCH] 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>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>
>Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
>Autobuild-Date(master): Mon Jun 29 14:00:24 CEST 2015 on sn-devel-104
>
>(cherry picked from commit eaf99203093cabc3069f1c69345d38d739b0663d)
>---
> 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 de254e9..5083d042 100644
>--- a/source3/winbindd/winbindd_dual.c
>+++ b/source3/winbindd/winbindd_dual.c
>@@ -117,6 +117,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;
>@@ -128,6 +129,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,
>@@ -151,6 +155,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;
> }
> 
>@@ -171,6 +178,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));
> }
>@@ -184,15 +193,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;
> 	}
>@@ -212,6 +217,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
>

Flags:

asn: review+

Actions: View

Attachments on bug 11358: 11194 | 11214 | 11215