[PATCH 1/2] s3/rpc_server: don't unmarshall PDUs twice

David Disseldorp ddiss at samba.org
Tue Dec 10 07:34:26 MST 2013


DCE/RPC PDUs are currently unmarshalled firstly by the generic librpc
dcerpc_read_ncacn_packet_[send/recv] functions, and subsequently a
second time by the source3 rpc_server, which ignores the unmarshalled
packet and re-parses the receive buffer.

Signed-off-by: David Disseldorp <ddiss at samba.org>
---
 source3/rpc_server/rpc_server.c | 59 +++++++++++++++++------------------------
 source3/rpc_server/rpc_server.h |  2 +-
 source3/rpc_server/srv_pipe.c   | 37 +++++++++++++++-----------
 3 files changed, 46 insertions(+), 52 deletions(-)

diff --git a/source3/rpc_server/rpc_server.c b/source3/rpc_server/rpc_server.c
index f283559..d10d3ac 100644
--- a/source3/rpc_server/rpc_server.c
+++ b/source3/rpc_server/rpc_server.c
@@ -432,9 +432,6 @@ void named_pipe_packet_process(struct tevent_req *subreq)
 	DATA_BLOB recv_buffer = data_blob_null;
 	struct ncacn_packet *pkt;
 	NTSTATUS status;
-	ssize_t data_left;
-	ssize_t data_used;
-	char *data;
 	uint32_t to_send;
 	size_t i;
 	bool ok;
@@ -445,23 +442,20 @@ void named_pipe_packet_process(struct tevent_req *subreq)
 		goto fail;
 	}
 
-	data_left = recv_buffer.length;
-	data = (char *)recv_buffer.data;
-
-	while (data_left) {
-
-		data_used = process_incoming_data(npc->p, data, data_left);
-		if (data_used < 0) {
-			DEBUG(3, ("Failed to process dceprc request!\n"));
-			status = NT_STATUS_UNEXPECTED_IO_ERROR;
-			goto fail;
-		}
-
-		data_left -= data_used;
-		data += data_used;
+	/* dcerpc_read_ncacn_packet_recv() returns a full PDU */
+	npc->p->in_data.pdu_needed_len = 0;
+	npc->p->in_data.pdu = recv_buffer;
+	if (dcerpc_get_endian_flag(&recv_buffer) & DCERPC_DREP_LE) {
+		npc->p->endian = RPC_LITTLE_ENDIAN;
+	} else {
+		npc->p->endian = RPC_BIG_ENDIAN;
 	}
+	DEBUG(10, ("PDU is in %s Endian format!\n",
+		   npc->p->endian ? "Big" : "Little"));
+	process_complete_pdu(npc->p, pkt);
 
-	/* Do not leak this buffer, npc is a long lived context */
+	/* reset pipe state and free PDU */
+	npc->p->in_data.pdu.length = 0;
 	talloc_free(recv_buffer.data);
 	talloc_free(pkt);
 
@@ -1134,10 +1128,7 @@ static void dcerpc_ncacn_packet_process(struct tevent_req *subreq)
 	struct _output_data *out = &ncacn_conn->p->out_data;
 	DATA_BLOB recv_buffer = data_blob_null;
 	struct ncacn_packet *pkt;
-	ssize_t data_left;
-	ssize_t data_used;
 	uint32_t to_send;
-	char *data;
 	NTSTATUS status;
 	bool ok;
 
@@ -1153,22 +1144,20 @@ static void dcerpc_ncacn_packet_process(struct tevent_req *subreq)
 		goto fail;
 	}
 
-	data_left = recv_buffer.length;
-	data = (char *) recv_buffer.data;
-
-	while (data_left) {
-		data_used = process_incoming_data(ncacn_conn->p, data, data_left);
-		if (data_used < 0) {
-			DEBUG(3, ("Failed to process dcerpc request!\n"));
-			status = NT_STATUS_UNEXPECTED_IO_ERROR;
-			goto fail;
-		}
-
-		data_left -= data_used;
-		data += data_used;
+	/* dcerpc_read_ncacn_packet_recv() returns a full PDU */
+	ncacn_conn->p->in_data.pdu_needed_len = 0;
+	ncacn_conn->p->in_data.pdu = recv_buffer;
+	if (dcerpc_get_endian_flag(&recv_buffer) & DCERPC_DREP_LE) {
+		ncacn_conn->p->endian = RPC_LITTLE_ENDIAN;
+	} else {
+		ncacn_conn->p->endian = RPC_BIG_ENDIAN;
 	}
+	DEBUG(10, ("PDU is in %s Endian format!\n",
+		   ncacn_conn->p->endian ? "Big" : "Little"));
+	process_complete_pdu(ncacn_conn->p, pkt);
 
-	/* Do not leak this buffer */
+	/* reset pipe state and free PDU */
+	ncacn_conn->p->in_data.pdu.length = 0;
 	talloc_free(recv_buffer.data);
 	talloc_free(pkt);
 
diff --git a/source3/rpc_server/rpc_server.h b/source3/rpc_server/rpc_server.h
index d204a8b..0d7bdb5 100644
--- a/source3/rpc_server/rpc_server.h
+++ b/source3/rpc_server/rpc_server.h
@@ -77,7 +77,7 @@ int make_server_pipes_struct(TALLOC_CTX *mem_ctx,
 			     int *perrno);
 
 void set_incoming_fault(struct pipes_struct *p);
-void process_complete_pdu(struct pipes_struct *p);
+void process_complete_pdu(struct pipes_struct *p, struct ncacn_packet *pkt);
 int create_named_pipe_socket(const char *pipe_name);
 bool setup_named_pipe_socket(const char *pipe_name,
 			     struct tevent_context *ev_ctx,
diff --git a/source3/rpc_server/srv_pipe.c b/source3/rpc_server/srv_pipe.c
index f92de3b..7a25d6e 100644
--- a/source3/rpc_server/srv_pipe.c
+++ b/source3/rpc_server/srv_pipe.c
@@ -1490,21 +1490,21 @@ static bool process_request_pdu(struct pipes_struct *p, struct ncacn_packet *pkt
  Processes a finished PDU stored in p->in_data.pdu.
 ****************************************************************************/
 
-void process_complete_pdu(struct pipes_struct *p)
+/* TODO remove */
+static void handle_complete_pdu(struct pipes_struct *p)
 {
 	struct ncacn_packet *pkt = NULL;
 	NTSTATUS status;
-	bool reply = False;
 
 	if(p->fault_state) {
 		DEBUG(10,("RPC connection in fault state.\n"));
-		goto done;
+		goto err_fault;
 	}
 
 	pkt = talloc(p->mem_ctx, struct ncacn_packet);
 	if (!pkt) {
 		DEBUG(0, ("Out of memory!\n"));
-		goto done;
+		goto err_fault;
 	}
 
 	/*
@@ -1523,9 +1523,24 @@ void process_complete_pdu(struct pipes_struct *p)
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0, ("Failed to unmarshal rpc packet: %s!\n",
 			  nt_errstr(status)));
-		goto done;
+		goto err_fault;
 	}
 
+	handle_complete_pdu(p, pkt);
+
+	TALLOC_FREE(pkt);
+	return;
+
+err_fault:
+	DEBUG(3,("DCE/RPC fault sent!"));
+	set_incoming_fault(p);
+	setup_fault_pdu(p, NT_STATUS(DCERPC_FAULT_OP_RNG_ERROR));
+}
+
+void process_complete_pdu(struct pipes_struct *p, struct ncacn_packet *pkt)
+{
+	bool reply = false;
+
 	/* Store the call_id */
 	p->call_id = pkt->call_id;
 
@@ -1644,21 +1659,11 @@ void process_complete_pdu(struct pipes_struct *p)
 		break;
 	}
 
-done:
 	if (!reply) {
 		DEBUG(3,("DCE/RPC fault sent!"));
 		set_incoming_fault(p);
 		setup_fault_pdu(p, NT_STATUS(DCERPC_FAULT_OP_RNG_ERROR));
-		TALLOC_FREE(pkt);
-	} else {
-		/*
-		 * Reset the lengths. We're ready for a new pdu.
-		 */
-		TALLOC_FREE(p->in_data.pdu.data);
-		p->in_data.pdu_needed_len = 0;
-		p->in_data.pdu.length = 0;
 	}
-
-	TALLOC_FREE(pkt);
+	/* pkt and p->in_data.pdu.data freed by caller */
 }
 
-- 
1.8.1.4



More information about the samba-technical mailing list