Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:FrontRunner
tomcat.28365
tomcat-9.0.36-CVE-2021-43980.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File tomcat-9.0.36-CVE-2021-43980.patch of Package tomcat.28365
From 170e0f792bd18ff031677890ba2fe50eb7a376c1 Mon Sep 17 00:00:00 2001 From: Mark Thomas <markt@apache.org> Date: Tue, 29 Mar 2022 19:15:37 +0100 Subject: [PATCH] Improve the recycling of Processor objects to make it more robust. --- java/org/apache/coyote/AbstractProtocol.java | 32 ++++++++++--------- .../tomcat/util/net/SocketWrapperBase.java | 17 +++++++--- webapps/docs/changelog.xml | 4 +++ 3 files changed, 33 insertions(+), 20 deletions(-) Index: apache-tomcat-9.0.36-src/java/org/apache/coyote/AbstractProtocol.java =================================================================== --- apache-tomcat-9.0.36-src.orig/java/org/apache/coyote/AbstractProtocol.java +++ apache-tomcat-9.0.36-src/java/org/apache/coyote/AbstractProtocol.java @@ -775,7 +775,11 @@ public abstract class AbstractProtocol<S S socket = wrapper.getSocket(); - Processor processor = (Processor) wrapper.getCurrentProcessor(); + // We take complete ownership of the Processor inside of this method to ensure + // no other thread can release it while we're using it. Whatever processor is + // held by this variable will be associated with the SocketWrapper before this + // method returns. + Processor processor = (Processor) wrapper.takeCurrentProcessor(); if (getLog().isDebugEnabled()) { getLog().debug(sm.getString("abstractConnectionHandler.connectionsGet", processor, socket)); @@ -860,9 +864,6 @@ public abstract class AbstractProtocol<S processor.setSslSupport( wrapper.getSslSupport(getProtocol().getClientCertProvider())); - // Associate the processor with the connection - wrapper.setCurrentProcessor(processor); - SocketState state = SocketState.CLOSED; do { state = processor.process(wrapper, status); @@ -882,8 +883,6 @@ public abstract class AbstractProtocol<S release(processor); // Create the upgrade processor processor = upgradeProtocol.getProcessor(wrapper, getProtocol().getAdapter()); - // Associate with the processor with the connection - wrapper.setCurrentProcessor(processor); } else { if (getLog().isDebugEnabled()) { getLog().debug(sm.getString( @@ -906,8 +905,6 @@ public abstract class AbstractProtocol<S wrapper.unRead(leftOverInput); // Mark the connection as upgraded wrapper.setUpgraded(true); - // Associate with the processor with the connection - wrapper.setCurrentProcessor(processor); // Initialise the upgrade handler (which may trigger // some IO using the new protocol which is why the lines // above are necessary) @@ -945,8 +942,8 @@ public abstract class AbstractProtocol<S } else if (state == SocketState.OPEN) { // In keep-alive but between requests. OK to recycle // processor. Continue to poll for the next request. - wrapper.setCurrentProcessor(null); release(processor); + processor = null; wrapper.registerReadInterest(); } else if (state == SocketState.SENDFILE) { // Sendfile in progress. If it fails, the socket will be @@ -971,8 +968,7 @@ public abstract class AbstractProtocol<S // Connection closed. OK to recycle the processor. // Processors handling upgrades require additional clean-up // before release. - wrapper.setCurrentProcessor(null); - if (processor.isUpgrade()) { + if (processor != null && processor.isUpgrade()) { UpgradeToken upgradeToken = processor.getUpgradeToken(); HttpUpgradeHandler httpUpgradeHandler = upgradeToken.getHttpUpgradeHandler(); InstanceManager instanceManager = upgradeToken.getInstanceManager(); @@ -993,7 +989,13 @@ public abstract class AbstractProtocol<S } } } + release(processor); + processor = null; + } + + if (processor != null) { + wrapper.setCurrentProcessor(processor); } return state; } catch(java.net.SocketException e) { @@ -1031,7 +1033,6 @@ public abstract class AbstractProtocol<S // Make sure socket/processor is removed from the list of current // connections - wrapper.setCurrentProcessor(null); release(processor); return SocketState.CLOSED; } @@ -1065,7 +1066,9 @@ public abstract class AbstractProtocol<S /** * Expected to be used by the handler once the processor is no longer - * required. + * required. Care must be taken to ensure that this method is only + * called once per processor, after the request processing has + * completed. * * @param processor Processor being released (that was associated with * the socket) @@ -1104,8 +1107,7 @@ public abstract class AbstractProtocol<S */ @Override public void release(SocketWrapperBase<S> socketWrapper) { - Processor processor = (Processor) socketWrapper.getCurrentProcessor(); - socketWrapper.setCurrentProcessor(null); + Processor processor = (Processor) socketWrapper.takeCurrentProcessor(); release(processor); } Index: apache-tomcat-9.0.36-src/java/org/apache/tomcat/util/net/SocketWrapperBase.java =================================================================== --- apache-tomcat-9.0.36-src.orig/java/org/apache/tomcat/util/net/SocketWrapperBase.java +++ apache-tomcat-9.0.36-src/java/org/apache/tomcat/util/net/SocketWrapperBase.java @@ -30,6 +30,7 @@ import java.util.concurrent.RejectedExec import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -105,10 +106,12 @@ public abstract class SocketWrapperBase< protected volatile OperationState<?> writeOperation = null; /** - * The org.apache.coyote.Processor instance currently associated - * with the wrapper. + * The org.apache.coyote.Processor instance currently associated with the + * wrapper. Only populated when required to maintain wrapper<->Processor + * mapping between calls to + * {@link AbstractEndpoint.Handler#process(SocketWrapperBase, SocketEvent)}. */ - protected Object currentProcessor = null; + private final AtomicReference<Object> currentProcessor = new AtomicReference<>(); public SocketWrapperBase(E socket, AbstractEndpoint<E,?> endpoint) { this.socket = socket; @@ -135,11 +138,15 @@ public abstract class SocketWrapperBase< } public Object getCurrentProcessor() { - return currentProcessor; + return currentProcessor.get(); } public void setCurrentProcessor(Object currentProcessor) { - this.currentProcessor = currentProcessor; + this.currentProcessor.set(currentProcessor); + } + + public Object takeCurrentProcessor() { + return currentProcessor.getAndSet(null); } /** Index: apache-tomcat-9.0.36-src/webapps/docs/changelog.xml =================================================================== --- apache-tomcat-9.0.36-src.orig/webapps/docs/changelog.xml +++ apache-tomcat-9.0.36-src/webapps/docs/changelog.xml @@ -130,6 +130,10 @@ Ensure that the HTTP/1.1 processor is correctly recycled when a direct connection to h2c is made. (markt) </fix> + <fix> + Improve the recycling of Processor objects to make it more robust. + (markt) + </fix> </changelog> </subsection> <subsection name="Jasper">
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
