From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 4 Apr 2019 04:25:30 +0000 (+1300)
Subject: ldap_server: Use an array of struct iovec to avoid data_blob_append()
X-Git-Tag: ldb-2.0.5~754
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=00b9a97706e71e6834781df0a144f23af9526689;p=thirdparty%2Fsamba.git

ldap_server: Use an array of struct iovec to avoid data_blob_append()

This avoids a the implicit 256MB limit on LDAP replies (allowing this
to be increased in the future) and means we copy less memory around.

However because we can only have 1024 entries in a struct iovec (on Linux)
we will need to call tstream_writev_queue_send() multiple times.

Calling it in chunks of 1024 seems a reasonable compromise, the
gensec layer will chunk it out smaller if required.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---

diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c
index d0be35972b0..53e9af94888 100644
--- a/source4/ldap_server/ldap_server.c
+++ b/source4/ldap_server/ldap_server.c
@@ -672,30 +672,31 @@ static void ldapsrv_call_wait_done(struct tevent_req *subreq)
 static void ldapsrv_call_writev_start(struct ldapsrv_call *call)
 {
 	struct ldapsrv_connection *conn = call->conn;
-	DATA_BLOB blob = data_blob_null;
+	struct ldapsrv_reply *reply = NULL;
 	struct tevent_req *subreq = NULL;
+	size_t length = 0;
+	size_t i;
 
-	/* build all the replies into a single blob */
-	while (call->replies) {
-		bool ret;
+	call->iov_count = 0;
 
-		ret = data_blob_append(call,
-				       &blob,
-				       call->replies->blob.data,
-				       call->replies->blob.length);
-		data_blob_free(&call->replies->blob);
+	/* build all the replies into an IOV (no copy) */
+	for (reply = call->replies;
+	     reply != NULL;
+	     reply = reply->next) {
+		/*
+		 * Overflow is harmless here, just used below to
+		 * decide if to read or write
+		 */
+		length += reply->blob.length;
 
-		if (!ret) {
-			ldapsrv_terminate_connection(conn, "data_blob_append failed");
-			return;
-		}
-
-		talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet");
-
-		DLIST_REMOVE(call->replies, call->replies);
+		/*
+		 * At worst an overflow would mean we send less
+		 * replies
+		 */
+		call->iov_count++;
 	}
 
-	if (blob.length == 0) {
+	if (length == 0) {
 		if (!call->notification.busy) {
 			TALLOC_FREE(call);
 		}
@@ -704,14 +705,48 @@ static void ldapsrv_call_writev_start(struct ldapsrv_call *call)
 		return;
 	}
 
-	call->out_iov.iov_base = blob.data;
-	call->out_iov.iov_len = blob.length;
+	/* Cap call->iov_count at IOV_MAX */
+	call->iov_count = MIN(call->iov_count, IOV_MAX);
+
+	call->out_iov = talloc_array(call,
+				     struct iovec,
+				     call->iov_count);
+	if (!call->out_iov) {
+		/* This is not ideal */
+		ldapsrv_terminate_connection(conn,
+					     "failed to allocate "
+					     "iovec array");
+		return;
+	}
+
+	/* We may have had to cap the number of replies at IOV_MAX */
+	for (i = 0;
+	     i < call->iov_count && call->replies != NULL;
+	     i++) {
+		reply = call->replies;
+		call->out_iov[i].iov_base = reply->blob.data;
+		call->out_iov[i].iov_len = reply->blob.length;
+
+		/* Keep only the ASN.1 encoded data */
+		talloc_steal(call->out_iov, reply->blob.data);
+
+		DLIST_REMOVE(call->replies, reply);
+		TALLOC_FREE(reply);
+	}
+
+	if (i > call->iov_count) {
+		/* This is not ideal, but also (essentially) impossible */
+		ldapsrv_terminate_connection(conn,
+					     "call list ended"
+					     "before iov_count");
+		return;
+	}
 
 	subreq = tstream_writev_queue_send(call,
 					   conn->connection->event.ctx,
 					   conn->sockets.active,
 					   conn->sockets.send_queue,
-					   &call->out_iov, 1);
+					   call->out_iov, call->iov_count);
 	if (subreq == NULL) {
 		ldapsrv_terminate_connection(conn, "stream_writev_queue_send failed");
 		return;
@@ -732,6 +767,9 @@ static void ldapsrv_call_writev_done(struct tevent_req *subreq)
 
 	rc = tstream_writev_queue_recv(subreq, &sys_errno);
 	TALLOC_FREE(subreq);
+
+	/* This releases the ASN.1 encoded packets from memory */
+	TALLOC_FREE(call->out_iov);
 	if (rc == -1) {
 		const char *reason;
 
@@ -762,6 +800,12 @@ static void ldapsrv_call_writev_done(struct tevent_req *subreq)
 		return;
 	}
 
+	/* Perhaps still some more to send */
+	if (call->replies != NULL) {
+		ldapsrv_call_writev_start(call);
+		return;
+	}
+
 	if (!call->notification.busy) {
 		TALLOC_FREE(call);
 	}
diff --git a/source4/ldap_server/ldap_server.h b/source4/ldap_server/ldap_server.h
index 9f21da512f9..75c8adcb5ef 100644
--- a/source4/ldap_server/ldap_server.h
+++ b/source4/ldap_server/ldap_server.h
@@ -72,7 +72,8 @@ struct ldapsrv_call {
 		struct ldap_message *msg;
 		DATA_BLOB blob;
 	} *replies;
-	struct iovec out_iov;
+	struct iovec *out_iov;
+	size_t iov_count;
 
 	struct tevent_req *(*wait_send)(TALLOC_CTX *mem_ctx,
 					struct tevent_context *ev,