svn commit: samba r10699 - in branches/SAMBA_4_0/source/librpc/rpc: .

tridge at samba.org tridge at samba.org
Tue Oct 4 00:43:17 GMT 2005


Author: tridge
Date: 2005-10-04 00:43:16 +0000 (Tue, 04 Oct 2005)
New Revision: 10699

WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=10699

Log:

fixed the dcerpc code so that you can shutdown the pipe safely from
within a callback on the pipe. This should fix a problem volker
encountered with winbind. The fix invoolves making the recv_data
handler free the memory for a packet, instead of having the transport
layer free it after calling recv_data. When the transport layer freed
it, it had no way of knowing if the callback had shutdown the pipe, so
it had no way of knowing if it could safely use the pointer.

Also changed the pipe shutdown hook for the smb transport to use an
async SMB close. This ensures that when you shutdown the pipe, you
don't block waiting for the server to ack the close of the pipe fnum.


Modified:
   branches/SAMBA_4_0/source/librpc/rpc/dcerpc.c
   branches/SAMBA_4_0/source/librpc/rpc/dcerpc_smb.c
   branches/SAMBA_4_0/source/librpc/rpc/dcerpc_sock.c


Changeset:
Modified: branches/SAMBA_4_0/source/librpc/rpc/dcerpc.c
===================================================================
--- branches/SAMBA_4_0/source/librpc/rpc/dcerpc.c	2005-10-03 23:54:44 UTC (rev 10698)
+++ branches/SAMBA_4_0/source/librpc/rpc/dcerpc.c	2005-10-04 00:43:16 UTC (rev 10699)
@@ -756,6 +756,8 @@
 /*
   process a fragment received from the transport layer during a
   request
+
+  This function frees the data 
 */
 static void dcerpc_request_recv_data(struct dcerpc_connection *c, 
 				     DATA_BLOB *data,
@@ -766,6 +768,8 @@
 	uint_t length;
 	
 	if (!NT_STATUS_IS_OK(status)) {
+		data_blob_free(data);
+
 		/* all pending requests get the error */
 		while (c->pending) {
 			req = c->pending;
@@ -781,7 +785,7 @@
 
 	pkt.call_id = 0;
 
-	status = ncacn_pull_request_sign(c, data, (TALLOC_CTX *)data->data, &pkt);
+	status = ncacn_pull_request_sign(c, data, data->data, &pkt);
 
 	/* find the matching request. Notice we match before we check
 	   the status.  this is ok as a pending call_id can never be
@@ -792,29 +796,20 @@
 
 	if (req == NULL) {
 		DEBUG(2,("dcerpc_request: unmatched call_id %u in response packet\n", pkt.call_id));
+		data_blob_free(data);
 		return;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
 		req->status = status;
-		req->state = RPC_REQUEST_DONE;
-		DLIST_REMOVE(c->pending, req);
-		if (req->async.callback) {
-			req->async.callback(req);
-		}
-		return;
+		goto req_done;
 	}
 
 	if (pkt.ptype == DCERPC_PKT_FAULT) {
 		DEBUG(5,("rpc fault: %s\n", dcerpc_errstr(c, pkt.u.fault.status)));
 		req->fault_code = pkt.u.fault.status;
 		req->status = NT_STATUS_NET_WRITE_FAULT;
-		req->state = RPC_REQUEST_DONE;
-		DLIST_REMOVE(c->pending, req);
-		if (req->async.callback) {
-			req->async.callback(req);
-		}
-		return;
+		goto req_done;
 	}
 
 	if (pkt.ptype != DCERPC_PKT_RESPONSE) {
@@ -822,12 +817,7 @@
 			 (int)pkt.ptype)); 
 		req->fault_code = DCERPC_FAULT_OTHER;
 		req->status = NT_STATUS_NET_WRITE_FAULT;
-		req->state = RPC_REQUEST_DONE;
-		DLIST_REMOVE(c->pending, req);
-		if (req->async.callback) {
-			req->async.callback(req);
-		}
-		return;
+		goto req_done;
 	}
 
 	length = pkt.u.response.stub_and_verifier.length;
@@ -839,12 +829,7 @@
 						   req->payload.length + length);
 		if (!req->payload.data) {
 			req->status = NT_STATUS_NO_MEMORY;
-			req->state = RPC_REQUEST_DONE;
-			DLIST_REMOVE(c->pending, req);
-			if (req->async.callback) {
-				req->async.callback(req);
-			}
-			return;
+			goto req_done;
 		}
 		memcpy(req->payload.data+req->payload.length, 
 		       pkt.u.response.stub_and_verifier.data, length);
@@ -853,19 +838,22 @@
 
 	if (!(pkt.pfc_flags & DCERPC_PFC_FLAG_LAST)) {
 		c->transport.send_read(c);
+		data_blob_free(data);
 		return;
 	}
 
-	/* we've got the full payload */
-	req->state = RPC_REQUEST_DONE;
-	DLIST_REMOVE(c->pending, req);
-
 	if (!(pkt.drep[0] & DCERPC_DREP_LE)) {
 		req->flags |= DCERPC_PULL_BIGENDIAN;
 	} else {
 		req->flags &= ~DCERPC_PULL_BIGENDIAN;
 	}
 
+
+req_done:
+	/* we've got the full payload */
+	req->state = RPC_REQUEST_DONE;
+	DLIST_REMOVE(c->pending, req);
+	data_blob_free(data);
 	if (req->async.callback) {
 		req->async.callback(req);
 	}

Modified: branches/SAMBA_4_0/source/librpc/rpc/dcerpc_smb.c
===================================================================
--- branches/SAMBA_4_0/source/librpc/rpc/dcerpc_smb.c	2005-10-03 23:54:44 UTC (rev 10698)
+++ branches/SAMBA_4_0/source/librpc/rpc/dcerpc_smb.c	2005-10-04 00:43:16 UTC (rev 10699)
@@ -89,9 +89,11 @@
 	frag_length = dcerpc_get_frag_length(&state->data);
 
 	if (frag_length <= state->received) {
-		state->data.length = state->received;
-		state->c->transport.recv_data(state->c, &state->data, NT_STATUS_OK);
+		DATA_BLOB data = state->data;
+		data.length = state->received;
+		talloc_steal(state->c, data.data);
 		talloc_free(state);
+		state->c->transport.recv_data(state->c, &data, NT_STATUS_OK);
 		return;
 	}
 
@@ -204,8 +206,10 @@
 	}
 
 	if (!NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) {
-		c->transport.recv_data(c, &state->trans->out.data, NT_STATUS_OK);
+		DATA_BLOB data = state->trans->out.data;
+		talloc_steal(c, data.data);
 		talloc_free(state);
+		c->transport.recv_data(c, &data, NT_STATUS_OK);
 		return;
 	}
 
@@ -257,6 +261,8 @@
 	state->req->async.fn = smb_trans_callback;
 	state->req->async.private = state;
 
+	talloc_steal(state, state->req);
+
         return NT_STATUS_OK;
 }
 
@@ -322,6 +328,7 @@
 {
 	struct smb_private *smb = c->transport.private;
 	union smb_close io;
+	struct smbcli_request *req;
 
 	/* maybe we're still starting up */
 	if (!smb) return NT_STATUS_OK;
@@ -329,7 +336,11 @@
 	io.close.level = RAW_CLOSE_CLOSE;
 	io.close.in.fnum = smb->fnum;
 	io.close.in.write_time = 0;
-	smb_raw_close(smb->tree, &io);
+	req = smb_raw_close_send(smb->tree, &io);
+	if (req != NULL) {
+		/* we don't care if this fails, so just free it if it succeeds */
+		req->async.fn = (void (*)(struct smbcli_request *))talloc_free;
+	}
 
 	talloc_free(smb);
 

Modified: branches/SAMBA_4_0/source/librpc/rpc/dcerpc_sock.c
===================================================================
--- branches/SAMBA_4_0/source/librpc/rpc/dcerpc_sock.c	2005-10-03 23:54:44 UTC (rev 10698)
+++ branches/SAMBA_4_0/source/librpc/rpc/dcerpc_sock.c	2005-10-04 00:43:16 UTC (rev 10699)
@@ -120,6 +120,7 @@
 	struct sock_private *sock = p->transport.private;
 	NTSTATUS status;
 	size_t nread;
+	DATA_BLOB data;
 
 	if (sock->recv.data.data == NULL) {
 		sock->recv.data = data_blob_talloc(sock, NULL, MIN_HDR_SIZE);
@@ -176,14 +177,15 @@
 	}
 
 	/* we have a full packet */
-	p->transport.recv_data(p, &sock->recv.data, NT_STATUS_OK);
-	talloc_free(sock->recv.data.data);
+	data = sock->recv.data;
 	sock->recv.data = data_blob(NULL, 0);
 	sock->recv.received = 0;
 	sock->recv.pending_count--;
 	if (sock->recv.pending_count == 0) {
 		EVENT_FD_NOT_READABLE(sock->fde);
 	}
+
+	p->transport.recv_data(p, &data, NT_STATUS_OK);
 }
 
 /*



More information about the samba-cvs mailing list