https://bz.apache.org/bugzilla/show_bug.cgi?id=70311

            Bug ID: 70311
           Summary: SecureNioChannel and SecureNio2Channel single buffer
                    reads loop forever on BUFFER_OVERFLOW when expand()
                    does not increase the free space
           Product: Tomcat 9
           Version: 9.0.122
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Connectors
          Assignee: [email protected]
          Reporter: [email protected]
  Target Milestone: -----

Created attachment 40245
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=40245&action=edit
Patch for 9.0.x, 10.1.x and 11.0.x (git format-patch)

SecureNioChannel.read(ByteBuffer) loops forever when SSLEngine.unwrap()
returns BUFFER_OVERFLOW and expand() cannot add free space to the
destination buffer. If the buffer is already at the application buffer
size, SocketBufferHandler.expand() returns the same buffer and
Http11InputBuffer.expand() only moves the limit. When the free space is
still less than one TLS record, every unwrap() overflows again and the
do/while loop never exits. One core runs at 100% until the JVM restarts.
No read timeout fires, because the loop never returns to the socket.

The vectored overload read(ByteBuffer[], int, int) already guards this
case. It sets a "resized" flag after expand() and throws IOException
when the buffer did not change (added with d58aa0849f in 2019). The
single buffer overload has no equivalent check. Both single buffer reads
in SecureNio2Channel, FutureRead and the CompletionHandler read, have
the same loop. main has no NIO2, so that part applies to 11.0.x, 10.1.x
and 9.0.x.

We hit it on 9.0.122 with Http11NioProtocol, JSSE and OpenJDK 17. The
trigger was an application bug like the one in Bug 64771. A stale reader
reset position on a buffer already handed to the next request on the
same keep-alive connection. That left position 2600, limit 16680 after
expand() and capacity 24896, so 14080 bytes were free for a 16384-byte
record. Two heap dumps 8 hours apart showed the identical buffer state.
We fixed the application bug on our side. Any such buffer state, from
any cause, pins a core instead of failing one request.

Expected: the read fails with IOException, as the vectored overload does.
Actual: infinite loop.

Proposed fix, attached as git format-patch (one for main, one for
9.0.x/10.1.x/11.0.x): record dst.remaining() before expand() and throw
IOException only if unwrap() consumed nothing and the free space did not
grow. JSSE consumes nothing on BUFFER_OVERFLOW, so the check catches
every JSSE spin. OpenSSL consumes input on overflow, so the check does
not change its recovery path. We verified that with the FFM OpenSSL
engine on 11.0.x. The NIO2 Future read also gets the
getAppReadBufHandler() != null check the other reads have. New message:
channel.nio.ssl.unwrapFailNoRoom.

The patches add unit tests, TestSecureNioChannelOverflow and
TestSecureNio2ChannelOverflow. Each test writes one TLS record into
netInBuffer with no socket I/O. Without the fix, the overflow tests time
out on main, 11.0.x, 10.1.x and 9.0.x, compiled at each branch's
compile.release. With the fix, all pass. The existing util/net and
coyote/http11 tests give the same results with and without the patch.
The patches apply with git am to the current tips (main 21b5789258,
9.0.x 74c36224f7). The main patch includes a changelog entry. A pull
request against main will follow.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to