Package: apt-cacher-ng
Version: 3.7.5-1
Tags: patch, memory-leak
X-Debbugs-CC: [email protected], [email protected]
We've recently upgraded our apt-cacher-ng from Debian Bullseye (acng
3.6.4-1) to Trixie and noticed unbounded memory leakage which resulted
in OOM (32 GB RAM) in ~36 hours (20260413_14h47m21s_apt-cacher-ng.png).
We've mitigated the issue by restarting the service in regular
intervals, but I wasn't happy with this workaround.
cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 13 (trixie)"
NAME="Debian GNU/Linux"
VERSION_ID="13"
VERSION="13 (trixie)"
VERSION_CODENAME=trixie
DEBIAN_VERSION_FULL=13.4
ID=debian
HOME_URL="https://www.debian.org/";
SUPPORT_URL="https://www.debian.org/support";
BUG_REPORT_URL="https://bugs.debian.org/";
uname -a
Linux filehost01 6.12.74+deb13+1-amd64 #1 SMP PREEMPT_DYNAMIC Debian
6.12.74-2 (2026-03-08) x86_64 GNU/Linux
Since I'm not a C++ expert, I've decided to burn some tokens and tasked
Github Copilot CLI (via Claude Opus) to analyze the commits between
3.6.4 .. 3.7.5 + the repository code in general for potential leaks. I
will attach the four patches for potential issues that Copilot / Claude
Opus came up with. The tcpconnect.cc SSL leak seems to be the main
culprit (we use some apt sources on our hosts via
http://acnghost:3124/HTTPS///, which also executed apt-get update very
often). Also in 2019 (commit: 527c1dece5aa56c53f019872ace6e6ebbb74238a)
the comparison of the DNS cache (dns_exp_q.front()->second->m_expTime <=
now) seems to have been inverted accidentally.
After rebuilding from source with the attached patches applied, memory
usage is much reduced (20260414_10h02m37s_apt-cacher-ng-patched.png).
There might still be some very minor leakage (RSS went up from 20288 to
55540 over night), but it seems quite stable now at this value. It could
be worthwhile to dig deeper using valgrind and such, but I don't have
the setup for that right now.
*Disclosure:* The provided patches have been authored in their entirety
by machine learning and casually reviewed by me (the last time I did
anything with C++ is probably 10 years ago). I hope there's no double
free() now; the SSL BIO stuff goes over my head a bit. At the very
least, it hasn't produced a crash yet.
Since the AI/LLM sphere is a hot button topic in the community, feel
free to disregard the proposed patches and you could recognize this mail
as a mere a bug report.
Best regards,
Sebastian Döring
Senior System Administrator
Infrastructure Server and Service Operations
1&1 Telecommunication SE | Hinterm Hauptbahnhof 3 | 76137 Karlsruhe |
Deutschland
E-Mail: [email protected] | Web: www.1und1.de <
https://www.1und1.de >
Die gesetzlichen Pflichtangaben finden Sie unter
https://unternehmen.1und1.de/unternehmen/impressum/.
Member of United Internet
Diese E-Mail kann vertrauliche und/oder gesetzlich geschützte
Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat
sind oder diese E-Mail irrtümlich erhalten haben, unterrichten Sie bitte
den Absender und vernichten Sie diese E-Mail. Anderen als dem
bestimmungsgemäßen Adressaten ist untersagt, diese E-Mail zu speichern,
weiterzuleiten oder ihren Inhalt auf welche Weise auch immer zu verwenden.
This e-mail may contain confidential and/or privileged information. If
you are not the intended recipient of this e-mail, you are hereby
notified that saving, distribution or use of the content of this e-mail
in any way is prohibited. If you have received this e-mail in error,
please notify the sender and delete the e-mail.
From 46bf935dc1bb936759974148ac5c664e816af3be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20D=C3=B6ring?= <[email protected]>
Date: Mon, 13 Apr 2026 13:11:16 +0200
Subject: [PATCH 4/7] Bug 4: Fix tDnsResContext leak on bad DNS configuration
(HIGH SEVERITY)
When DNS resolver is not properly configured (resolver is nullptr),
a tDnsResContext was allocated with 'new' but never freed on the
error path before returning.
Fix: Add 'delete ctx' in the error handling block before return.
Co-authored-by: Copilot <[email protected]>
---
src/caddrinfo.cc | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/caddrinfo.cc b/src/caddrinfo.cc
index db500b9..0d0361e 100644
--- a/src/caddrinfo.cc
+++ b/src/caddrinfo.cc
@@ -443,9 +443,13 @@ void CAddrInfo::Resolve(cmstring & sHostname, uint16_t nPort, tDnsResultReporter
rep = decltype (rep)();
if (AC_UNLIKELY(!ctx || !ctx->resolver))
{
- ctx->cbs.back()(make_shared<CAddrInfo>("503 Bad DNS configuration"));
- // and defuse the fallback caller
- ctx->cbs.clear();
+ if (ctx)
+ {
+ ctx->cbs.back()(make_shared<CAddrInfo>("503 Bad DNS configuration"));
+ // and defuse the fallback caller
+ ctx->cbs.clear();
+ delete ctx;
+ }
return;
}
ctx->refMe = tDnsResContext::g_active_resolver_index.emplace(key, ctx).first;
--
2.47.3
From 4c55ab45ec04bf3975906263fd47fa3ce29cdec7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20D=C3=B6ring?= <[email protected]>
Date: Mon, 13 Apr 2026 12:21:28 +0200
Subject: [PATCH 3/7] Bug 3: DNS Cache Cleanup Logic Inverted (MEDIUM SEVERITY)
File: src/caddrinfo.cc line 179
// WRONG: removes NON-expired entries, keeps expired ones!
while(... && dns_exp_q.front()->second->m_expTime >= now)
// Should be: <= now
---
src/caddrinfo.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/caddrinfo.cc b/src/caddrinfo.cc
index 0ead6dd..db500b9 100644
--- a/src/caddrinfo.cc
+++ b/src/caddrinfo.cc
@@ -176,7 +176,7 @@ void CAddrInfo::clean_dns_cache()
auto now=GetTime();
ASSERT(dns_cache.size() == dns_exp_q.size());
while(dns_cache.size() >= DNS_CACHE_MAX-1
- || (!dns_exp_q.empty() && dns_exp_q.front()->second->m_expTime >= now))
+ || (!dns_exp_q.empty() && dns_exp_q.front()->second->m_expTime <= now))
{
dns_cache.erase(dns_exp_q.front());
dns_exp_q.pop_front();
--
2.47.3
From 163cbedd52ccb55019c82984d3ca1441e84feaa4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20D=C3=B6ring?= <[email protected]>
Date: Mon, 13 Apr 2026 12:21:10 +0200
Subject: [PATCH 2/7] Bug 2: lint_ptr Move Assignment Leak (HIGH SEVERITY)
File: src/acsmartptr.h
The move assignment operator fails to decrement the old pointer's reference count:
// BUGGY - old pointer never freed!
lint_ptr<T>& operator=(lint_ptr<T> &&other) {
if(m_ptr == other.m_ptr) return *this;
m_ptr = other.m_ptr; // LEAK: old m_ptr never released
other.m_ptr = nullptr;
return *this;
}
Root cause: The lint_ptr class is entirely new in 3.7.5 (introduced with c-ares DNS migration). Same bug exists in lint_user_ptr.
---
src/acsmartptr.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/acsmartptr.h b/src/acsmartptr.h
index 3cdc578..8b23fab 100644
--- a/src/acsmartptr.h
+++ b/src/acsmartptr.h
@@ -103,7 +103,9 @@ public:
{
if(m_ptr == other.m_ptr)
return *this;
-
+ // Must release old reference before taking new one
+ if(m_ptr)
+ m_ptr->__dec_ref();
m_ptr = other.m_ptr;
other.m_ptr = nullptr;
return *this;
@@ -231,6 +233,12 @@ public:
{
if(m_ptr == other.m_ptr)
return *this;
+ // Must release old reference before taking new one
+ if(m_ptr)
+ {
+ m_ptr->__dec_strong_ref();
+ m_ptr->__dec_ref();
+ }
m_ptr = other.m_ptr;
other.m_ptr = nullptr;
return *this;
--
2.47.3
From 0dc13aff4a1b4134a737908d75ef994eda7ba17f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20D=C3=B6ring?= <[email protected]>
Date: Mon, 13 Apr 2026 13:11:09 +0200
Subject: [PATCH 1/7] Bug 1: Fix SSL connection memory leak (CRITICAL SEVERITY)
SSL objects created with SSL_new() were never freed because:
- BIO_set_ssl() used BIO_NOCLOSE flag, preventing BIO cleanup from freeing SSL
- No SSL_free() call existed anywhere in the codebase
Fix:
- Change BIO_NOCLOSE to BIO_CLOSE so BIO_free_all() frees the SSL object
- Add SSL_free(ssl) in error handler lambda for early-return error paths
- Store SSL pointer in m_ssl member for later use after ownership transfer
Each leaked SSL object is ~30-50KB. With modern mirrors using HTTPS,
this is likely the primary cause of the observed ~800KB/s memory growth.
Co-authored-by: Copilot <[email protected]>
---
src/tcpconnect.cc | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/tcpconnect.cc b/src/tcpconnect.cc
index 64483c2..a58e1a9 100644
--- a/src/tcpconnect.cc
+++ b/src/tcpconnect.cc
@@ -83,6 +83,7 @@ void tcpconnect::Disconnect()
#ifdef HAVE_SSL
if(m_bio)
BIO_free_all(m_bio), m_bio=nullptr;
+ m_ssl = nullptr;
#endif
m_lastFile.reset();
@@ -310,10 +311,11 @@ bool tcpconnect::SSLinit(mstring &sErr)
SSL * ssl(nullptr);
mstring ebuf;
- auto withSslHeadPfx = [&sErr](const char *perr)
+ auto withSslHeadPfx = [&sErr, &ssl](const char *perr)
{
sErr="SSL error: ";
sErr+=(perr?perr:"Generic SSL failure");
+ if (ssl) SSL_free(ssl);
return false;
};
auto withLastSslError = [&withSslHeadPfx]()
@@ -341,7 +343,7 @@ bool tcpconnect::SSLinit(mstring &sErr)
}
ssl = SSL_new(m_ctx);
- if (!m_ctx) return withLastSslError();
+ if (!ssl) return withLastSslError();
bool disableNameValidation = cfg::nsafriendly == 1;// || (bGuessedTls * cfg::nsafriendly == 2);
bool disableAllValidation = cfg::nsafriendly == 1; // || (bGuessedTls * (cfg::nsafriendly == 2 || cfg::nsafriendly == 3));
@@ -415,7 +417,11 @@ bool tcpconnect::SSLinit(mstring &sErr)
// not sure we need it but maybe the handshake can access this data
BIO_set_conn_hostname(m_bio, m_sHostName.c_str());
BIO_set_conn_port(m_bio, tPortFmter().fmt(m_nPort));
- BIO_set_ssl(m_bio, ssl, BIO_NOCLOSE);
+ BIO_set_ssl(m_bio, ssl, BIO_CLOSE);
+ // ssl ownership transferred to m_bio, store reference for validation below
+ // but don't free it in error handler anymore (BIO_free_all handles it now)
+ m_ssl = ssl;
+ ssl = nullptr;
BIO_set_nbio(m_bio, 1);
set_nb(m_conFd);
@@ -423,10 +429,10 @@ bool tcpconnect::SSLinit(mstring &sErr)
if(!disableAllValidation)
{
X509* server_cert = nullptr;
- hret=SSL_get_verify_result(ssl);
+ hret=SSL_get_verify_result(m_ssl);
if( hret != X509_V_OK)
return withSslHeadPfx(X509_verify_cert_error_string(hret));
- server_cert = SSL_get_peer_certificate(ssl);
+ server_cert = SSL_get_peer_certificate(m_ssl);
if(server_cert)
{
// XXX: maybe extract the real name to a buffer and report it additionally?
--
2.47.3
