On 3/23/23 19:56, Hamilton Coutinho wrote:
We found a way to reproduce the leak: set up squid to run in intercept
mode + SSL description + memory pools off (to ease identifying the leak)
and then generate requests to sites with invalid certs (eg, CA not
installed), for instance: curl -k https://slscr.update.microsoft.com
Thanks a lot for sharing those details! Using them, I was able to
reproduce one memory leak. Please try the following fix:
https://github.com/squid-cache/squid/pull/1346
I do not know whether that PR changes port to v5 cleanly or whether the
same bug exists in v5. I can only speculate that v5 has a similar leak.
squidclient mgr:mem should show ever increasing HttpRequest instances.
As far as I can tell, the HttpRequest object created
in ConnStateData::buildFakeRequest() is never freed because its refcount
> 0.
Any ideas where an HTTPMSGUNLOCK() might be missing?
Sorry, I do not know. In my master/v7-based tests, HttpRequest objects
are not leaking. However, I am not testing an intercepting Squid. I
still have one red flag to investigate, so perhaps I will be able to
reproduce and fix that HttpRequest leak as well.
HTH,
Alex.
On Wed, Jan 18, 2023 at 11:28 AM Hamilton Coutinho wrote:
Hi Alex,
Thanks for the prompt reply! Thanks also for the clarifications.
Agreed, I just realized the requests seem to be failing
with Http::scServiceUnavailable, so my focus turned
to Security::PeerConnector::sslCrtvdHandleReply() and friends.
Best.
On Wed, Jan 18, 2023 at 11:11 AM Alex Rousskov
<rouss...@measurement-factory.com
<mailto:rouss...@measurement-factory.com>> wrote:
On 1/18/23 13:46, Hamilton Coutinho wrote:
> Hi all,
>
> We are observing what seems to be several objects leaking in
the output
> mgr:mem, to the tune of 10s of 1000s
>
of HttpRequest, HttpHeaderEntry, Comm::Connection,
Security::ErrorDetail, cbdata PeekingPeerConnector (31), etc.
>
> We dumped a core and managed to find some HttpRequest objects
and they
> all seem to have failed in the same way, with an
ERR_SECURE_CONNECT_FAIL
> category, for a site that has a certificate signed by a CA
authority not
> available to squid.
>
> If I would guess, the origin of the problem might be in
> Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched():
>
> if (finalAction == Ssl::bumpTerminate) {
> bail(new ErrorState(ERR_SECURE_CONNECT_FAIL,
Http::scForbidden,
> request.getRaw(), al));
> clientConn->close();
> clientConn = nullptr;
>
> Wondering if assigning null to clientConn there would be
premature.
FWIW, that connection pointer reset itself looks OK to me.
ConnStateData
and/or others should have a connection closure handler attached
to the
clientConn descriptor. That handler should be notified by Comm and
initiate cleanup of the objects responsible for client-Squid
communication.
The bail() call above should inform the requestor about the
error/termination and terminate this AsyncJob. That requestor
should
then close the Squid-server connection and clean up associated
state.
While there may be bugs in those "should..." sequences, please
note that
the pasted code is not related to handling of untrusted origin
servers
(unless your ssl_bump rules specifically activate the terminate
action
upon discovering such an origin server). The pasted code is
reacting to
an "ssl_bump terminate" rule matching.
Cheers,
Alex.
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
<mailto:squid-dev@lists.squid-cache.org>
http://lists.squid-cache.org/listinfo/squid-dev
<http://lists.squid-cache.org/listinfo/squid-dev>
--
Hamilton
--
Hamilton
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
