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

diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 5bb7cd0edbc20de70dd052c74d84e575e63e6a88..350ec7d277885520cf3f9fc24ce8941091a383dc 100644 (file)
--- 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;