From: Amos Jeffries <squid3@treenet.co.nz>
Date: Fri, 17 Dec 2010 18:56:56 +0000 (-0700)
Subject: Author: Graham Keeling <graham@equiinet.com>
X-Git-Tag: SQUID_3_1_10~30
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46459ba6eacf79da86b743db1b9afd4f66b9991c;p=thirdparty%2Fsquid.git

Author: Graham Keeling <graham@equiinet.com>
Bug 3113: Squid can eat far too much memory when uploading files

Problem description:
  Uploading a large file to a web site on the internet, squid's client
input buffer will increase far faster than it can be emptied to
the target website, and the machine will swiftly run out of memory.

This patch adds the client_request_buffer_max_size configuration
parameter  which specifies the maximum buffer size of a client request.
---

diff --git a/doc/release-notes/release-3.1.sgml b/doc/release-notes/release-3.1.sgml
index 9629f5ae66..518f7ad4f4 100644
--- a/doc/release-notes/release-3.1.sgml
+++ b/doc/release-notes/release-3.1.sgml
@@ -563,6 +563,10 @@ This section gives a thorough account of those changes in three categories:
 	direct client address in delay pools.
 	</verb>
 
+	<tag>client_request_buffer_max_size</tag>
+	<p>New directive added with squid-3.1.10 to set limits on the amount of buffer space allocated
+	for receiving upload and request data from clients.
+
         <tag>dns_v4_fallback</tag>
         <p>New option to prevent Squid from always looking up IPv4 regardless of whether IPv6 addresses are found.
            Squid will follow a policy of prefering IPv6 links, keeping the IPv4 only as a safety net behind IPv6.
diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 203d621abd..8a795e59c4 100644
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -736,6 +736,14 @@ configDoConfigure(void)
     }
 
 #endif
+
+    // prevent infinite fetch loops in the request parser
+    // due to buffer full but not enough data recived to finish parse
+    if (Config.maxRequestBufferSize <= Config.maxRequestHeaderSize) {
+        fatalf("Client request buffer of %u bytes cannot hold a request with %u bytes of headers." \
+               " Change client_request_buffer_max or request_header_max_size limits.",
+               (uint32_t)Config.maxRequestBufferSize, (uint32_t)Config.maxRequestHeaderSize);
+    }
 }
 
 /* Parse a time specification from the config file.  Store the
diff --git a/src/cf.data.pre b/src/cf.data.pre
index 55b82219b5..6a2d76c846 100644
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -3541,6 +3541,17 @@ DOC_START
 	be no limit imposed.
 DOC_END
 
+NAME: client_request_buffer_max_size
+COMMENT: (bytes)
+TYPE: b_size_t
+DEFAULT: 512 KB
+LOC: Config.maxRequestBufferSize
+DOC_START
+	This specifies the maximum buffer size of a client request.
+	It prevents squid eating too much memory when somebody uploads
+	a large file.
+DOC_END
+
 NAME: chunked_request_body_max_size
 COMMENT: (bytes)
 TYPE: b_int64_t
diff --git a/src/client_side.cc b/src/client_side.cc
index 8f28eb0879..a7d6c61f6b 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -207,7 +207,8 @@ ConnStateData::readSomeData()
 
     debugs(33, 4, "clientReadSomeData: FD " << fd << ": reading request...");
 
-    makeSpaceAvailable();
+    if (!maybeMakeSpaceAvailable())
+        return;
 
     typedef CommCbMemFunT<ConnStateData, CommIoCbParams> Dialer;
     reader = JobCallback(33, 5,
@@ -2153,13 +2154,22 @@ ConnStateData::getAvailableBufferLength() const
     return result;
 }
 
-void
-ConnStateData::makeSpaceAvailable()
+bool
+ConnStateData::maybeMakeSpaceAvailable()
 {
     if (getAvailableBufferLength() < 2) {
-        in.buf = (char *)memReallocBuf(in.buf, in.allocatedSize * 2, &in.allocatedSize);
+        size_t newSize;
+        if (in.allocatedSize >= Config.maxRequestBufferSize) {
+            debugs(33, 4, "request buffer full: client_request_buffer_max_size=" << Config.maxRequestBufferSize);
+            return false;
+        }
+        if ((newSize=in.allocatedSize * 2) > Config.maxRequestBufferSize) {
+            newSize=Config.maxRequestBufferSize;
+        }
+        in.buf = (char *)memReallocBuf(in.buf, newSize, &in.allocatedSize);
         debugs(33, 2, "growing request buffer: notYetUsed=" << in.notYetUsed << " size=" << in.allocatedSize);
     }
+    return true;
 }
 
 void
@@ -2873,7 +2883,10 @@ ConnStateData::handleRequestBodyData()
 void
 ConnStateData::noteMoreBodySpaceAvailable(BodyPipe::Pointer )
 {
-    handleRequestBodyData();
+    if (!handleRequestBodyData())
+        return;
+
+    readSomeData();
 }
 
 void
diff --git a/src/client_side.h b/src/client_side.h
index 0ec5ce5d5f..97ce655a4d 100644
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -139,7 +139,7 @@ public:
     bool areAllContextsForThisConnection() const;
     void freeAllContexts();
     void readNextRequest();
-    void makeSpaceAvailable();
+    bool maybeMakeSpaceAvailable();
     ClientSocketContext::Pointer getCurrentContext() const;
     void addContextToQueue(ClientSocketContext * context);
     int getConcurrentRequestCount() const;
diff --git a/src/structs.h b/src/structs.h
index 8aaeabea82..42d2e82e2c 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -186,6 +186,7 @@ struct SquidConfig {
     size_t maxRequestHeaderSize;
     int64_t maxRequestBodySize;
     int64_t maxChunkedRequestBodySize;
+    size_t maxRequestBufferSize;
     size_t maxReplyHeaderSize;
     acl_size_t *ReplyBodySize;