You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/02/10 17:03:51 UTC

[tomcat] branch 9.0.x updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 5a3c6f4  Fix BZ 65776. Reduce false positives detecting duplicate accept bug
5a3c6f4 is described below

commit 5a3c6f46568086e1e2503bb7fb0d48b9356e4217
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 10 16:02:05 2022 +0000

    Fix BZ 65776. Reduce false positives detecting duplicate accept bug
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
 java/org/apache/tomcat/util/net/AprEndpoint.java  | 22 ++++++++++++++++------
 java/org/apache/tomcat/util/net/Nio2Endpoint.java | 16 ++++++++++++----
 java/org/apache/tomcat/util/net/NioEndpoint.java  | 18 +++++++++++++-----
 webapps/docs/changelog.xml                        |  4 ++++
 4 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 01c2b77..6d0f10c 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -62,6 +62,7 @@ import org.apache.tomcat.jni.Status;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.buf.ByteBufferUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.Acceptor.AcceptorState;
 import org.apache.tomcat.util.net.openssl.OpenSSLContext;
@@ -114,6 +115,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
 
     private int previousAcceptedPort = -1;
     private String previousAcceptedAddress = null;
+    private long previouspreviousAcceptedSocketNanoTime = 0;
+
 
     // ------------------------------------------------------------ Constructor
 
@@ -125,8 +128,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
         setUseAsyncIO(false);
     }
 
-    // ------------------------------------------------------------- Properties
 
+    // ------------------------------------------------------------- Properties
 
     /**
      * Defer accept.
@@ -806,13 +809,20 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
             // Do the duplicate accept check here rather than in serverSocketaccept()
             // so we can cache the results in the SocketWrapper
             AprSocketWrapper wrapper = new AprSocketWrapper(socket, this);
-            if (wrapper.getRemotePort() == previousAcceptedPort) {
-                if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-                    throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+            // Bug does not affect Windows. Skip the check on that platform.
+            if (!JrePlatform.IS_WINDOWS) {
+                long currentNanoTime = System.nanoTime();
+                if (wrapper.getRemotePort() == previousAcceptedPort) {
+                    if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
+                        if (currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
+                            throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+                        }
+                    }
                 }
+                previousAcceptedPort = wrapper.getRemotePort();
+                previousAcceptedAddress = wrapper.getRemoteAddr();
+                previouspreviousAcceptedSocketNanoTime = currentNanoTime;
             }
-            previousAcceptedPort = wrapper.getRemotePort();
-            previousAcceptedAddress = wrapper.getRemoteAddr();
 
             connections.put(socket, wrapper);
             wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 0f6d8a0..f82f738 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.Acceptor.AcceptorState;
 import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,6 +86,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
     private SynchronizedStack<Nio2Channel> nioChannels;
 
     private SocketAddress previousAcceptedSocketRemoteAddress = null;
+    private long previouspreviousAcceptedSocketNanoTime = 0;
 
 
     // ------------------------------------------------------------- Properties
@@ -373,11 +375,17 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
     protected AsynchronousSocketChannel serverSocketAccept() throws Exception {
         AsynchronousSocketChannel result = serverSock.accept().get();
 
-        SocketAddress currentRemoteAddress = result.getRemoteAddress();
-        if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-            throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+        // Bug does not affect Windows. Skip the check on that platform.
+        if (!JrePlatform.IS_WINDOWS) {
+            SocketAddress currentRemoteAddress = result.getRemoteAddress();
+            long currentNanoTime = System.nanoTime();
+            if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+                    currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
+                throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+            }
+            previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+            previouspreviousAcceptedSocketNanoTime = currentNanoTime;
         }
-        previousAcceptedSocketRemoteAddress = currentRemoteAddress;
 
         return result;
     }
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 66b6636..196df26 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -58,6 +58,7 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.collections.SynchronizedQueue;
 import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.compat.JreCompat;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.Acceptor.AcceptorState;
 import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -109,11 +110,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
     private SynchronizedStack<NioChannel> nioChannels;
 
     private SocketAddress previousAcceptedSocketRemoteAddress = null;
+    private long previouspreviousAcceptedSocketNanoTime = 0;
 
 
     // ------------------------------------------------------------- Properties
 
-
     /**
      * Use System.inheritableChannel to obtain channel from stdin/stdout.
      */
@@ -543,11 +544,18 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
     @Override
     protected SocketChannel serverSocketAccept() throws Exception {
         SocketChannel result = serverSock.accept();
-        SocketAddress currentRemoteAddress = result.getRemoteAddress();
-        if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-            throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+
+        // Bug does not affect Windows. Skip the check on that platform.
+        if (!JrePlatform.IS_WINDOWS) {
+            SocketAddress currentRemoteAddress = result.getRemoteAddress();
+            long currentNanoTime = System.nanoTime();
+            if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+                    currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
+                throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+            }
+            previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+            previouspreviousAcceptedSocketNanoTime = currentNanoTime;
         }
-        previousAcceptedSocketRemoteAddress = currentRemoteAddress;
 
         return result;
     }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b5182aa..b7bfc09 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,10 @@
         ignored when the Connector used an internal executor. (markt)
       </fix>
       <fix>
+        <bug>65776</bug>: Improve the detection of the Linux duplicate accept
+        bug and reduce (hopefully avoid) instances of false positives. (markt)
+      </fix>
+      <fix>
         <bug>65848</bug>: Revert the change that attempted to align the
         behaviour of client certificate authentication with NIO or NIO2 with
         OpenSSL for TLS between MacOS and Linux/Windows as the root cause was

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org