Adjust NS_CLIENT_TCP_BUFFER_SIZE and cleanup client_allocsendbuf

NS_CLIENT_TCP_BUFFER_SIZE was 2 byte too large following the
move to netmgr add associated changes to lib/ns/client.c and
as a result an INSIST could be trigger if the DNS message being
constructed had a checkpoint stage that fell in those two extra
bytes.  Adjusted NS_CLIENT_TCP_BUFFER_SIZE and cleaned up
client_allocsendbuf now that the previously reserved 2 bytes
are no longer used.

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 8a959b229e7e1160977be207327d771850f75809..1c8df4b0fb0f5133d5b83072c5c8edeca148c186 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -287,45 +287,20 @@ client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
        isc_nmhandle_unref(handle);
 }
 
-/*%
- * We only want to fail with ISC_R_NOSPACE when called from
- * ns_client_sendraw() and not when called from ns_client_send(),
- * tcpbuffer is NULL when called from ns_client_sendraw() and
- * length != 0.  tcpbuffer != NULL when called from ns_client_send()
- * and length == 0.
- */
-
-static isc_result_t
+static void
 client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
-                   isc_buffer_t *tcpbuffer, uint32_t length,
                    unsigned char **datap) {
        unsigned char *data;
        uint32_t bufsize;
-       isc_result_t result;
 
        REQUIRE(datap != NULL);
-       REQUIRE((tcpbuffer == NULL && length != 0) ||
-               (tcpbuffer != NULL && length == 0));
 
        if (TCP_CLIENT(client)) {
                INSIST(client->tcpbuf == NULL);
-               if (length + 2 > NS_CLIENT_TCP_BUFFER_SIZE) {
-                       result = ISC_R_NOSPACE;
-                       goto done;
-               }
                client->tcpbuf = isc_mem_get(client->mctx,
                                             NS_CLIENT_TCP_BUFFER_SIZE);
                data = client->tcpbuf;
-               if (tcpbuffer != NULL) {
-                       isc_buffer_init(tcpbuffer, data,
-                                       NS_CLIENT_TCP_BUFFER_SIZE);
-                       isc_buffer_init(buffer, data,
-                                       NS_CLIENT_TCP_BUFFER_SIZE);
-               } else {
-                       isc_buffer_init(buffer, data,
-                                       NS_CLIENT_TCP_BUFFER_SIZE);
-                       INSIST(length <= 0xffff);
-               }
+               isc_buffer_init(buffer, data, NS_CLIENT_TCP_BUFFER_SIZE);
        } else {
                data = client->sendbuf;
                if ((client->attributes & NS_CLIENTATTR_HAVECOOKIE) == 0) {
@@ -343,17 +318,9 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
                if (bufsize > NS_CLIENT_SEND_BUFFER_SIZE) {
                        bufsize = NS_CLIENT_SEND_BUFFER_SIZE;
                }
-               if (length > bufsize) {
-                       result = ISC_R_NOSPACE;
-                       goto done;
-               }
                isc_buffer_init(buffer, data, bufsize);
        }
        *datap = data;
-       result = ISC_R_SUCCESS;
-
-done:
-       return (result);
 }
 
 static isc_result_t
@@ -385,8 +352,10 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
                goto done;
        }
 
-       result = client_allocsendbuf(client, &buffer, NULL, mr->length, &data);
-       if (result != ISC_R_SUCCESS) {
+       client_allocsendbuf(client, &buffer, &data);
+
+       if (mr->length > isc_buffer_length(&buffer)) {
+               result = ISC_R_NOSPACE;
                goto done;
        }
 
@@ -422,7 +391,6 @@ ns_client_send(ns_client_t *client) {
        isc_result_t result;
        unsigned char *data;
        isc_buffer_t buffer = { .magic = 0 };
-       isc_buffer_t tcpbuffer = { .magic = 0 };
        isc_region_t r;
        dns_compress_t cctx;
        bool cleanup_cctx = false;
@@ -491,13 +459,7 @@ ns_client_send(ns_client_t *client) {
                }
        }
 
-       /*
-        * XXXRTH  The following doesn't deal with TCP buffer resizing.
-        */
-       result = client_allocsendbuf(client, &buffer, &tcpbuffer, 0, &data);
-       if (result != ISC_R_SUCCESS) {
-               goto done;
-       }
+       client_allocsendbuf(client, &buffer, &data);
 
        result = dns_compress_init(&cctx, -1, client->mctx);
        if (result != ISC_R_SUCCESS) {
@@ -619,7 +581,6 @@ renderend:
                client->sendcb(&buffer);
        } else if (TCP_CLIENT(client)) {
                isc_buffer_usedregion(&buffer, &r);
-               isc_buffer_add(&tcpbuffer, r.length);
 #ifdef HAVE_DNSTAP
                if (client->view != NULL) {
                        dns_dt_send(client->view, dtmsgtype, &client->peeraddr,
@@ -628,11 +589,10 @@ renderend:
                }
 #endif /* HAVE_DNSTAP */
 
-               /* don't count the 2-octet length header */
-               respsize = isc_buffer_usedlength(&tcpbuffer) - 2;
+               respsize = isc_buffer_usedlength(&buffer);
 
                isc_nmhandle_ref(client->handle);
-               result = client_sendpkg(client, &tcpbuffer);
+               result = client_sendpkg(client, &buffer);
                if (result != ISC_R_SUCCESS) {
                        /* We won't get a callback to clean it up */
                        isc_nmhandle_unref(client->handle);

diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 61612a32eacad24eb9212d55b2ea01e273ef88d3..74c435b0c61d0f213983a6135c791178b6870f51 100644 (file)
--- a/lib/ns/include/ns/client.h
+++ b/lib/ns/include/ns/client.h
@@ -81,7 +81,7 @@
  *** Types
  ***/
 
-#define NS_CLIENT_TCP_BUFFER_SIZE  (65535 + 2)
+#define NS_CLIENT_TCP_BUFFER_SIZE  65535
 #define NS_CLIENT_SEND_BUFFER_SIZE 4096
 
 /*!

diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c
index 9becd639a29186fc25d15a73bc4fa5b12e389551..b6e46578b5ac8eb6e510022aa1c1a2d990b53424 100644 (file)
--- a/lib/ns/xfrout.c
+++ b/lib/ns/xfrout.c
@@ -649,14 +649,13 @@ typedef struct {
        dns_db_t *db;
        dns_dbversion_t *ver;
        isc_quota_t *quota;
-       rrstream_t *stream;    /* The XFR RR stream */
-       bool question_added;   /* QUESTION section sent? */
-       bool end_of_stream;    /* EOS has been reached */
-       isc_buffer_t buf;      /* Buffer for message owner
-                               * names and rdatas */
-       isc_buffer_t txlenbuf; /* Transmit length buffer */
-       isc_buffer_t txbuf;    /* Transmit message buffer */
-       size_t cbytes;         /* Length of current message */
+       rrstream_t *stream;  /* The XFR RR stream */
+       bool question_added; /* QUESTION section sent? */
+       bool end_of_stream;  /* EOS has been reached */
+       isc_buffer_t buf;    /* Buffer for message owner
+                             * names and rdatas */
+       isc_buffer_t txbuf;  /* Transmit message buffer */
+       size_t cbytes;       /* Length of current message */
        void *txmem;
        unsigned int txmemlen;
        dns_tsigkey_t *tsigkey; /* Key used to create TSIG */
@@ -1269,12 +1268,11 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id,
 
        /*
         * Allocate another temporary buffer for the compressed
-        * response message and its TCP length prefix.
+        * response message.
         */
-       len = 2 + 65535;
+       len = NS_CLIENT_TCP_BUFFER_SIZE;
        mem = isc_mem_get(mctx, len);
-       isc_buffer_init(&xfr->txlenbuf, mem, 2);
-       isc_buffer_init(&xfr->txbuf, (char *)mem + 2, len - 2);
+       isc_buffer_init(&xfr->txbuf, (char *)mem, len);
        xfr->txmem = mem;
        xfr->txmemlen = len;
 
@@ -1324,7 +1322,6 @@ sendstream(xfrout_ctx_t *xfr) {
        int n_rrs;
 
        isc_buffer_clear(&xfr->buf);
-       isc_buffer_clear(&xfr->txlenbuf);
        isc_buffer_clear(&xfr->txbuf);
 
        is_tcp = ((xfr->client->attributes & NS_CLIENTATTR_TCP) != 0);