From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 15 Jul 2024 21:59:54 +0000 (+0000)
Subject: Do not drop queued TCP DNS queries when queuing a new one (#1863)
X-Git-Tag: SQUID_7_0_1~89
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4216816eb481fb484e6dc8b28dab11babb0ccd87;p=thirdparty%2Fsquid.git

Do not drop queued TCP DNS queries when queuing a new one (#1863)

Since 2005 commit 032785bf, previously queued serialized DNS query
(waiting for a TCP connection to the nameserver or for a write on an
already established TCP connection) was erased every time a new query
was serialized and appended to the `vc->queue` MemBuf. Thus, at most one
TCP query was submitted when the nameserver became available.
Non-serialized versions of erased queries remained on the DNS lru_list
and would eventually be retried or timeout. This change restores
conditional MemBuf initialization that was replaced with an
unconditional MemBuf reset in that 2005 commit.

It would take more than 5 hours of 1000/s unique request load for
100-byte DNS queries to overflow a 2GB MemBuf in a properly functioning
Squid, but this change does not rely on MEM_BUF_MAX_SIZE always being
large enough. New code prevents MemBuf::grow() assertions and informs
the admin about dropped queries that may indicate a Squid bug or a
persistent problem with nameserver communication.
---

diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index f264301535..1dcad7cb77 100644
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -933,7 +933,20 @@ idnsSendQueryVC(idns_query * q, size_t nsn)
         return;
     }
 
-    vc->queue->reset();
+    if (vc->queue->isNull())
+        vc->queue->init();
+
+    const auto serialiedQuerySize = 2 + q->sz + 1; // payload_length + payload + terminate()
+    if (vc->queue->potentialSpaceSize() < serialiedQuerySize) {
+        // header + payload + MemBuf terminator exceed maximum space size
+        debugs(78, DBG_IMPORTANT, "ERROR: Dropping DNS query due to insufficient buffer space for DNS over TCP query queue" <<
+               Debug::Extra << "query: " << q->name <<
+               Debug::Extra << "nameserver: " << nameservers[nsn].S <<
+               Debug::Extra << "used space: " << vc->queue->contentSize() <<
+               Debug::Extra << "remaining space: " << vc->queue->potentialSpaceSize() <<
+               Debug::Extra << "required space: " << serialiedQuerySize);
+        return; // the query will timeout and either fail or be retried
+    }
 
     short head = htons(q->sz);