From a872005c1c725e571e7da263d75e634affd1f87d Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 4 Oct 2005 00:43:16 +0000 Subject: r10699: 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. (This used to be commit c87d7f580e39245db181605f50883de07dd9632e) --- source4/librpc/rpc/dcerpc.c | 46 +++++++++++++++------------------------- source4/librpc/rpc/dcerpc_smb.c | 19 +++++++++++++---- source4/librpc/rpc/dcerpc_sock.c | 6 ++++-- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c index 352972b0b7..2c0dbcd5f3 100644 --- a/source4/librpc/rpc/dcerpc.c +++ b/source4/librpc/rpc/dcerpc.c @@ -756,6 +756,8 @@ NTSTATUS dcerpc_bind_byuuid(struct dcerpc_pipe *p, /* 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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, 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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, 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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, 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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, (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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, 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 @@ static void dcerpc_request_recv_data(struct dcerpc_connection *c, 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); } diff --git a/source4/librpc/rpc/dcerpc_smb.c b/source4/librpc/rpc/dcerpc_smb.c index b63a3080f5..bc48d674bf 100644 --- a/source4/librpc/rpc/dcerpc_smb.c +++ b/source4/librpc/rpc/dcerpc_smb.c @@ -89,9 +89,11 @@ static void smb_read_callback(struct smbcli_request *req) 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 @@ static void smb_trans_callback(struct smbcli_request *req) } 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 @@ static NTSTATUS smb_send_trans_request(struct dcerpc_connection *c, DATA_BLOB *b 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 @@ static NTSTATUS smb_shutdown_pipe(struct dcerpc_connection *c) { 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 @@ static NTSTATUS smb_shutdown_pipe(struct dcerpc_connection *c) 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); diff --git a/source4/librpc/rpc/dcerpc_sock.c b/source4/librpc/rpc/dcerpc_sock.c index d7f87e9706..161c9a56dc 100644 --- a/source4/librpc/rpc/dcerpc_sock.c +++ b/source4/librpc/rpc/dcerpc_sock.c @@ -120,6 +120,7 @@ static void sock_process_recv(struct dcerpc_connection *p) 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 @@ static void sock_process_recv(struct dcerpc_connection *p) } /* 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); } /* -- cgit