[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Fri Aug 12 04:37:04 MDT 2011


The branch, master has been updated
       via  0d1a7fd s3:libsmb: keep the request order in cli_smb_req_unset_pending()
       via  edeb41a s3:libsmb: use tevent_req_defer_callback() unless there's only one request in cli_smb_received()
       via  02cb205 s3:libsmb: make use of cli_state_receive_next() in cli_smb_received()
       via  56d3c91 s3:libsmb: notify all request about failures in cli_smb_req_set_pending()
       via  4335a4f s3:libsmb: split out cli_state_receive_next() from cli_smb_req_set_pending()
       via  ca56711 s3:libsmb: use talloc_stackframe() in cli_smb_received()
       via  bae9324 s3:libsmb: call cli_smb_req_unset_pending() before tevent_req_done() also for chained requests
       via  d41d2e9 s3:libsmb: reset the destructor in cli_smb_req_unset_pending()
       via  c01b832 s3:libsmb: add cli_state_notify_pending() and use it
      from  6b3a12b s4-test: use standard process model for 'dc' server

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 0d1a7fda1ef5e1358228ab2b5e74b18a877e0732
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 12 08:39:15 2011 +0200

    s3:libsmb: keep the request order in cli_smb_req_unset_pending()
    
    metze
    
    Autobuild-User: Stefan Metzmacher <metze at samba.org>
    Autobuild-Date: Fri Aug 12 12:36:03 CEST 2011 on sn-devel-104

commit edeb41aa204ca9814602bee1c7d9be32e5f737b5
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:45:26 2011 +0200

    s3:libsmb: use tevent_req_defer_callback() unless there's only one request in cli_smb_received()
    
    Callers of tevent_req_done() (or similar functions) have to return directly.
    Otherwise the callback could invalidate the current stack state,
    which is likely to trigger segfaults.
    
    If there was only one pending request and we just got the response
    for that one, we can use tevent_req_done() directly.
    
    Otherwise there're more pending requests and we need to call
    cli_state_receive_next() or we got the response for chained requests.
    Both means that we have to use tevent_req_defer_callback().
    
    metze

commit 02cb2052d8c68a3ba6dc8a19f580f25039321a75
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:28:06 2011 +0200

    s3:libsmb: make use of cli_state_receive_next() in cli_smb_received()
    
    metze

commit 56d3c91af70809fbdd86de888ac983a41e070ca3
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:26:31 2011 +0200

    s3:libsmb: notify all request about failures in cli_smb_req_set_pending()
    
    It's up to the caller to notify the current request,
    but we have to notify all other pending requests if
    we're not able to read the next response from the server.
    
    metze

commit 4335a4f1c6ad4e8dfcd4cc1bcd8b3ec2f258a4d8
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:16:02 2011 +0200

    s3:libsmb: split out cli_state_receive_next() from cli_smb_req_set_pending()
    
    metze

commit ca567117b028d8954453585bfb753e7f01c98319
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:07:55 2011 +0200

    s3:libsmb: use talloc_stackframe() in cli_smb_received()
    
    metze

commit bae9324e50c7513b2cae20bb352dbd014444471c
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:18:58 2011 +0200

    s3:libsmb: call cli_smb_req_unset_pending() before tevent_req_done() also for chained requests
    
    metze

commit d41d2e93f4e13e7975bcd8d4b7dc125f81ef2559
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 12:18:26 2011 +0200

    s3:libsmb: reset the destructor in cli_smb_req_unset_pending()
    
    metze

commit c01b8326e05c4113e3e979e02061fbc47476dadd
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Aug 11 09:35:38 2011 +0200

    s3:libsmb: add cli_state_notify_pending() and use it
    
    If we got a problem on the connection we need to notify every
    pending request. But we need to make use of tevent_req_defer_callback()
    before tevent_req_nterror(), otherwise the callback, triggered
    by tevent_req_nterror(), could invalidate the state of current caller,
    which will likely cause segfaults.
    
    metze

-----------------------------------------------------------------------

Summary of changes:
 source3/libsmb/async_smb.c |  190 +++++++++++++++++++++++++++++++-------------
 1 files changed, 135 insertions(+), 55 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/libsmb/async_smb.c b/source3/libsmb/async_smb.c
index ecc7780..488e953 100644
--- a/source3/libsmb/async_smb.c
+++ b/source3/libsmb/async_smb.c
@@ -132,6 +132,8 @@ void cli_smb_req_unset_pending(struct tevent_req *req)
 		return;
 	}
 
+	talloc_set_destructor(req, NULL);
+
 	if (num_pending == 1) {
 		/*
 		 * The pending read_smb tevent_req is a child of
@@ -160,7 +162,9 @@ void cli_smb_req_unset_pending(struct tevent_req *req)
 	/*
 	 * Remove ourselves from the cli->conn.pending array
 	 */
-	cli->conn.pending[i] = cli->conn.pending[num_pending-1];
+	for (; i < (num_pending - 1); i++) {
+		cli->conn.pending[i] = cli->conn.pending[i+1];
+	}
 
 	/*
 	 * No NULL check here, we're shrinking by sizeof(void *), and
@@ -185,7 +189,8 @@ static int cli_smb_req_destructor(struct tevent_req *req)
 	return 0;
 }
 
-static void cli_smb_received(struct tevent_req *subreq);
+static bool cli_state_receive_next(struct cli_state *cli);
+static void cli_state_notify_pending(struct cli_state *cli, NTSTATUS status);
 
 bool cli_smb_req_set_pending(struct tevent_req *req)
 {
@@ -207,10 +212,40 @@ bool cli_smb_req_set_pending(struct tevent_req *req)
 	cli->conn.pending = pending;
 	talloc_set_destructor(req, cli_smb_req_destructor);
 
+	if (!cli_state_receive_next(cli)) {
+		/*
+		 * the caller should notify the current request
+		 *
+		 * And all other pending requests get notified
+		 * by cli_state_notify_pending().
+		 */
+		cli_smb_req_unset_pending(req);
+		cli_state_notify_pending(cli, NT_STATUS_NO_MEMORY);
+		return false;
+	}
+
+	return true;
+}
+
+static void cli_smb_received(struct tevent_req *subreq);
+
+static bool cli_state_receive_next(struct cli_state *cli)
+{
+	size_t num_pending = talloc_array_length(cli->conn.pending);
+	struct tevent_req *req;
+	struct cli_smb_state *state;
+
 	if (cli->conn.read_smb_req != NULL) {
 		return true;
 	}
 
+	if (num_pending == 0) {
+		return true;
+	}
+
+	req = cli->conn.pending[0];
+	state = tevent_req_data(req, struct cli_smb_state);
+
 	/*
 	 * We're the first ones, add the read_smb request that waits for the
 	 * answer from the server
@@ -218,13 +253,39 @@ bool cli_smb_req_set_pending(struct tevent_req *req)
 	cli->conn.read_smb_req = read_smb_send(cli->conn.pending, state->ev,
 					       cli->conn.fd);
 	if (cli->conn.read_smb_req == NULL) {
-		cli_smb_req_unset_pending(req);
 		return false;
 	}
 	tevent_req_set_callback(cli->conn.read_smb_req, cli_smb_received, cli);
 	return true;
 }
 
+static void cli_state_notify_pending(struct cli_state *cli, NTSTATUS status)
+{
+	cli_state_disconnect(cli);
+
+	/*
+	 * Cancel all pending requests. We do not do a for-loop walking
+	 * cli->conn.pending because that array changes in
+	 * cli_smb_req_destructor().
+	 */
+	while (talloc_array_length(cli->conn.pending) > 0) {
+		struct tevent_req *req;
+		struct cli_smb_state *state;
+
+		req = cli->conn.pending[0];
+		state = tevent_req_data(req, struct cli_smb_state);
+
+		cli_smb_req_unset_pending(req);
+
+		/*
+		 * we need to defer the callback, because we may notify more
+		 * then one caller.
+		 */
+		tevent_req_defer_callback(req, state->ev);
+		tevent_req_nterror(req, status);
+	}
+}
+
 /*
  * Fetch a smb request's mid. Only valid after the request has been sent by
  * cli_smb_req_send().
@@ -490,8 +551,8 @@ static void cli_smb_sent(struct tevent_req *subreq)
 	nwritten = writev_recv(subreq, &err);
 	TALLOC_FREE(subreq);
 	if (nwritten == -1) {
-		cli_state_disconnect(state->cli);
-		tevent_req_nterror(req, map_nt_error_from_unix(err));
+		NTSTATUS status = map_nt_error_from_unix(err);
+		cli_state_notify_pending(state->cli, status);
 		return;
 	}
 
@@ -522,6 +583,7 @@ static void cli_smb_received(struct tevent_req *subreq)
 {
 	struct cli_state *cli = tevent_req_callback_data(
 		subreq, struct cli_state);
+	TALLOC_CTX *frame = talloc_stackframe();
 	struct tevent_req *req;
 	struct cli_smb_state *state;
 	NTSTATUS status;
@@ -536,23 +598,27 @@ static void cli_smb_received(struct tevent_req *subreq)
 		DEBUG(1, ("Internal error: cli_smb_received called with "
 			  "unexpected subreq\n"));
 		status = NT_STATUS_INTERNAL_ERROR;
-		goto fail;
+		cli_state_notify_pending(cli, status);
+		return;
 	}
 
-	received = read_smb_recv(subreq, talloc_tos(), &inbuf, &err);
+	received = read_smb_recv(subreq, frame, &inbuf, &err);
 	TALLOC_FREE(subreq);
 	cli->conn.read_smb_req = NULL;
 	if (received == -1) {
-		cli_state_disconnect(cli);
 		status = map_nt_error_from_unix(err);
-		goto fail;
+		cli_state_notify_pending(cli, status);
+		TALLOC_FREE(frame);
+		return;
 	}
 
 	if ((IVAL(inbuf, 4) != 0x424d53ff) /* 0xFF"SMB" */
 	    && (SVAL(inbuf, 4) != 0x45ff)) /* 0xFF"E" */ {
 		DEBUG(10, ("Got non-SMB PDU\n"));
 		status = NT_STATUS_INVALID_NETWORK_RESPONSE;
-		goto fail;
+		cli_state_notify_pending(cli, status);
+		TALLOC_FREE(frame);
+		return;
 	}
 
 	if (cli_encryption_on(cli) && (CVAL(inbuf, 0) == 0)) {
@@ -562,7 +628,9 @@ static void cli_smb_received(struct tevent_req *subreq)
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("get_enc_ctx_num returned %s\n",
 				   nt_errstr(status)));
-			goto fail;
+			cli_state_notify_pending(cli, status);
+			TALLOC_FREE(frame);
+			return;
 		}
 
 		if (enc_ctx_num != cli->trans_enc_state->enc_ctx_num) {
@@ -570,7 +638,9 @@ static void cli_smb_received(struct tevent_req *subreq)
 				   enc_ctx_num,
 				   cli->trans_enc_state->enc_ctx_num));
 			status = NT_STATUS_INVALID_HANDLE;
-			goto fail;
+			cli_state_notify_pending(cli, status);
+			TALLOC_FREE(frame);
+			return;
 		}
 
 		status = common_decrypt_buffer(cli->trans_enc_state,
@@ -578,7 +648,9 @@ static void cli_smb_received(struct tevent_req *subreq)
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("common_decrypt_buffer returned %s\n",
 				   nt_errstr(status)));
-			goto fail;
+			cli_state_notify_pending(cli, status);
+			TALLOC_FREE(frame);
+			return;
 		}
 	}
 
@@ -592,7 +664,6 @@ static void cli_smb_received(struct tevent_req *subreq)
 	}
 	if (i == num_pending) {
 		/* Dump unexpected reply */
-		TALLOC_FREE(inbuf);
 		goto done;
 	}
 
@@ -610,7 +681,6 @@ static void cli_smb_received(struct tevent_req *subreq)
 
 		if (!oplock_break) {
 			/* Dump unexpected reply */
-			TALLOC_FREE(inbuf);
 			goto done;
 		}
 	}
@@ -621,65 +691,75 @@ static void cli_smb_received(struct tevent_req *subreq)
 	if (!oplock_break /* oplock breaks are not signed */
 	    && !cli_check_sign_mac(cli, (char *)inbuf, state->seqnum+1)) {
 		DEBUG(10, ("cli_check_sign_mac failed\n"));
-		TALLOC_FREE(inbuf);
 		status = NT_STATUS_ACCESS_DENIED;
-		cli_state_disconnect(cli);
-		goto fail;
+		cli_state_notify_pending(cli, status);
+		TALLOC_FREE(frame);
+		return;
 	}
 
 	if (state->chained_requests == NULL) {
 		state->inbuf = talloc_move(state, &inbuf);
-		talloc_set_destructor(req, NULL);
 		cli_smb_req_unset_pending(req);
 		state->chain_num = 0;
 		state->chain_length = 1;
+
+		if (talloc_array_length(cli->conn.pending) == 0) {
+			tevent_req_done(req);
+			TALLOC_FREE(frame);
+			return;
+		}
+
+		tevent_req_defer_callback(req, state->ev);
 		tevent_req_done(req);
 	} else {
-		struct tevent_req **chain = talloc_move(
-			talloc_tos(), &state->chained_requests);
+		struct tevent_req **chain = talloc_move(frame,
+					    &state->chained_requests);
 		int num_chained = talloc_array_length(chain);
 
+		/*
+		 * We steal the inbuf to the chain,
+		 * so that it will stay until all
+		 * requests of the chain are finished.
+		 *
+		 * Each requests in the chain will
+		 * hold a talloc reference to the chain.
+		 * This way we do not expose the talloc_reference()
+		 * behavior to the callers.
+		 */
+		talloc_steal(chain, inbuf);
+
 		for (i=0; i<num_chained; i++) {
-			state = tevent_req_data(chain[i], struct
-						cli_smb_state);
+			struct tevent_req **ref;
+
+			req = chain[i];
+			state = tevent_req_data(req, struct cli_smb_state);
+
+			cli_smb_req_unset_pending(req);
+
+			/*
+			 * as we finish multiple requests here
+			 * we need to defer the callbacks as
+			 * they could destroy our current stack state.
+			 */
+			tevent_req_defer_callback(req, state->ev);
+
+			ref = talloc_reference(state, chain);
+			if (tevent_req_nomem(ref, req)) {
+				continue;
+			}
+
 			state->inbuf = inbuf;
 			state->chain_num = i;
 			state->chain_length = num_chained;
-			tevent_req_done(chain[i]);
+
+			tevent_req_done(req);
 		}
-		TALLOC_FREE(inbuf);
-		TALLOC_FREE(chain);
 	}
  done:
-	if ((talloc_array_length(cli->conn.pending) > 0) &&
-	    (cli->conn.read_smb_req == NULL)) {
-		/*
-		 * Set up another read request for the other pending cli_smb
-		 * requests
-		 */
-		state = tevent_req_data(cli->conn.pending[0],
-					struct cli_smb_state);
-		cli->conn.read_smb_req = read_smb_send(
-			cli->conn.pending, state->ev, cli->conn.fd);
-		if (cli->conn.read_smb_req == NULL) {
-			status = NT_STATUS_NO_MEMORY;
-			goto fail;
-		}
-		tevent_req_set_callback(cli->conn.read_smb_req,
-					cli_smb_received, cli);
-	}
-	return;
- fail:
-	/*
-	 * Cancel all pending requests. We don't do a for-loop walking
-	 * cli->conn.pending because that array changes in
-	 * cli_smb_req_destructor().
-	 */
-	while (talloc_array_length(cli->conn.pending) > 0) {
-		req = cli->conn.pending[0];
-		talloc_set_destructor(req, NULL);
-		cli_smb_req_unset_pending(req);
-		tevent_req_nterror(req, status);
+	TALLOC_FREE(frame);
+
+	if (!cli_state_receive_next(cli)) {
+		cli_state_notify_pending(cli, NT_STATUS_NO_MEMORY);
 	}
 }
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list