http: throttle per-connection reads while a request is in flight #36123

pinheadmz commented at 9:00 PM on August 30, 2026: member

This patches a memory exhaustion scenario found while auditing the new http server with kimi-k3. A shallow version of this scenario was addressed in #35735 (See #35735 (review) and #35735 (comment)) but a OOM vector still remained.

On master when the sever is busy handling a request from a client, it will still read data from that client and "queue up" the next request. In #35735 we handled the scenario where that additional incoming data was an invalid HTTP request by not attempting to parse the data. However, we didn't add a size limit.

A misbehaving client could block its request queue with something like waitforblock and then flood the server with nonsense data without any limit.

The solution in this patch is to not even read from the socket at all if we are busy with a request. Similar to the intent of #35735, the kernel will buffer incoming data until backpressure kicks in and the TCP window drops to 0.

If unaddressed, the attack vector is still limited to authenticated clients: unauthenticated REST requests don't block for very long, so the server should be able to drain the receive buffer.

DrahtBot added the label RPC/REST/ZMQ on Aug 30, 2026

DrahtBot commented at 9:00 PM on August 30, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36123.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	winterrdog

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

pinheadmz commented at 9:01 PM on August 30, 2026: member

Requesting review from @frankomosh and @winterrdog who helped address the initial issue

DrahtBot added the label CI failed on Aug 30, 2026

pinheadmz force-pushed on Aug 30, 2026

pinheadmz commented at 11:37 PM on August 30, 2026: member

Trying another approach to de-flake-ify the test

in test/functional/interface_http.py:723 in 412e0c6e29

 718 | +        # one full request.
 719 | +        stuck_since = None
 720 | +        sent = 0
 721 | +        while True:
 722 | +            try:
 723 | +                sent += conn.conn.sock.send(flood)

hodlinator commented at 9:00 AM on August 31, 2026:

Since we are going through the trouble of actually creating a well formatted request, we might as well not garble the bytes:

                sent += conn.conn.sock.send(flood[sent % len(flood):])

in test/functional/interface_http.py:727 in 412e0c6e29

 722 | +            try:
 723 | +                sent += conn.conn.sock.send(flood)
 724 | +                # The server is still reading
 725 | +                stuck_since = None
 726 | +                self.log.debug(f"sent: {sent}")
 727 | +                assert sent <= len(flood), (

hodlinator commented at 10:26 AM on August 31, 2026:

Given the CI failures on Windows (https://github.com/bitcoin/bitcoin/actions/runs/33342301034/job/99340016146?pr=36123#step:14:1050), I think Windows has a differently sized TCP window (or possibly still buffers data on the client end?). Doubling the threshold fixed it for me during local testing. Maybe safer to go 10x.

                assert sent <= len(flood) * 2, (

in test/functional/interface_http.py:721 in 412e0c6e29 outdated

 716 | +        # nothing, so sends keep stalling; an unpatched server drains 64KB per
 717 | +        # I/O tick even while busy, so progress resumes and `sent` climbs past
 718 | +        # one full request.
 719 | +        stuck_since = None
 720 | +        sent = 0
 721 | +        while True:

hodlinator commented at 10:41 AM on August 31, 2026:

Would be good to set a bound on this loop, just in case it prevents the test from completing like in this Mac run: https://github.com/bitcoin/bitcoin/actions/runs/33342301034/job/99340016279?pr=36123

Something like:

--- a/test/functional/interface_http.py
+++ b/test/functional/interface_http.py
@@ -718,6 +718,7 @@ class HTTPBasicsTest (BitcoinTestFramework):
         # one full request.
         stuck_since = None
         sent = 0
+        start = time.time()
         while True:
             try:
                 sent += conn.conn.sock.send(flood)
@@ -740,6 +741,9 @@ class HTTPBasicsTest (BitcoinTestFramework):
                     # After 5 seconds of no progress we assume the server is
                     # behaving appropriately.
                     break
+            duration = time.time() - start
+            assert duration < 60 * self.options.timeout_factor, \
+                f"Failed to prove appropriate behavior after {duration:.0f} seconds."

         self.log.info(f"Pipelined flood stalled after {sent} bytes; no progress for 5s")

pinheadmz force-pushed on Aug 31, 2026

pinheadmz commented at 12:28 PM on August 31, 2026: member

push to 3e259fcc7d074212887a82c52b59250ee4ee5f58:

Took all suggestions from @hodlinator review, improving test and hopefully passing CI on windows. I reproduced the failed test and the fix on my own native windows machine. Still confused about the runaway macos test though since I wrote and tested the patch originally on macos!

winterrdog commented at 3:18 PM on August 31, 2026: contributor

Concept ACK

I like the idea of moving this kind of hard work away from the application layer, and then delegating it to the OS at the transport layer (usually the OS has handled a lot of strange edge cases over the years)

Will provide more review

http: throttle per-connection reads while a request is in flight

A client streaming pipelined requests into a busy connection
(or any connection whose replies are slower than the sender) could grow
server memory without limit, up to remote OOM.

Stop selecting RecvEvent for clients whose request is being processed;
pipelined data then backs up in the kernel socket buffer, applying TCP
backpressure to the sender. One request per connection is in flight
at a time.

Functional test streams pipelined submitblock requests into a connection
blocked on waitforblockheight. Unpatched builds continue draining the
socket buffer indefinitely, patched builds will stall.

782d195fc3

pinheadmz force-pushed on Aug 31, 2026

pinheadmz commented at 7:30 PM on August 31, 2026: member

push to 782d195fc370456d0f54bb9178a1c9ba9829ed08

Try again to pass the test on all platforms. It now exits in three possible places during the blocked-flood:

~3 GB read into memory by the server (FAIL)
The server is reading/draining data from the buffer for 60 seconds (FAIL)
Neither of the above occur AND 5 seconds pass with the server blocking (PASS)