Bug 5554: Cap absurd inherited RLIMIT_NOFILE

[mxschmitt]

Bug 5554: Squid can allocate huge memory from RLIMIT_NOFILE

Bug report: https://bugs.squid-cache.org/show_bug.cgi?id=5554

When Squid starts in a Kubernetes/containerd environment, it may inherit
an absurdly large RLIMIT_NOFILE soft limit (for example 1073741816).
Squid currently uses that value as Squid_MaxFD and eagerly allocates
several descriptor-sized tables during startup. The result is
approximately 536-568 GiB of virtual memory, high CPU, and an
OOM/unusable process before it can serve requests.

The same behavior reproduces with current Squid master (8.0.0-VCS) in an
empty Kubernetes pod. The packaged Squid 7.6 image reproduces it as
well. Outside Kubernetes, with a normal descriptor limit, the same image
starts normally.

Relevant allocation path:

• setMaxFD() assigns Squid_MaxFD from the inherited RLIMIT_NOFILE.
• fde::Init() allocates xcalloc(Squid_MaxFD, sizeof(fde)).
• Comm::MakeCallbackTable() allocates another descriptor-sized table.
• The later SQUID_MAXFD_LIMIT clamp occurs after these allocations.

This patch caps an inherited default above SQUID_MAXFD before those
allocations and emits a warning. Explicit max_filedescriptors
configuration remains honored, including values above SQUID_MAXFD where
supported.

Before:
RLIMIT_NOFILE=1073741816
VmSize approximately 562-595 GB
pod becomes unusable/OOMs

After:
warning reports the inherited limit and the 32768 cap
VmSize approximately 38 MB
VmRSS approximately 20 MB
pod remains Running

An explicit max_filedescriptors 1048576 regression check remains
honored.

Testing:

• Built current Squid master on Linux ARM64.
• Reproduced the failure in Kubernetes/containerd.
• Rebuilt with this patch and verified normal startup and memory usage
  in the same pod.
• Verified explicit max_filedescriptors behavior.
• git diff --check passes.

[kinkie]

Please restore the empty line at the end of the file.
Apart from this, looks good to me!

[rousskov]

Please restore the empty line at the end of the file.

I will fix this while working on the adjustments mentioned in another change request.

[rousskov]

Bug report: Squid should not blindly trust an absurd inherited soft limit and allocate unbounded descriptor tables before startup completes. It should cap an inherited default to a safe compiled/runtime limit, or otherwise reject it with a clear diagnostic.

Thank you for this bug report and analysis, Max! I agree with the above conclusion and support this PR direction. Some PR changes are required, but I will implement them myself in hope to reduce the number of review iterations and cumulative time spent on this PR. Please stand by -- the ball is in my court.

Since this is your first Squid contribution, I will also add your entry to the CONTRIBUTORS file (unless you stop me).

[rousskov]

This new code deals with so called "inherited" limit, but it is placed into the existing else clause where a non-inherited limit is also tested, necessitating unwanted Config.max_filedescriptors check duplication. The existing code is already messy, but it is easy to avoid making it worse AFAICT.

I will try to adjust this code to avoid this problem. It is easier to do that than to describe that adjustment.

[rousskov]

I assume Squid supports exceeding SQUID_MAXFD -- existing Config.max_filedescriptors code above does not check SQUID_MAXFD.

I fear that this new check will break many existing legitimate descriptor increases via "inherited ulimit" because SQUID_MAXFD is going to be too small in those cases. There is nothing about "absurd" (using PR title terminology) in this proposed PR code. The proposed C++ comment says "unexpectedly large", but if overwriting SQUID_MAXFD via ulimit is supported, then we cannot claim that all reasonable ulimit values are "unexpected".

While working on the adjustments mentioned in another change request, I plan to add what this PR title promises -- detection of absurdly large inherited RLIMIT_NOFILE value.

[rousskov]

We should not overwrite rl, especially if we do not call setrlimit() with the new/overwritten value. We should set Squid_MaxFD directly instead.

While working on the adjustments mentioned in another change request, I will address this concern.

[rousskov]

Please restore the empty line at the end of the file.

I will fix this while working on the adjustments mentioned in another change request.

[mxschmitt]

Thanks @rousskov for taking over - highly appreciate your time you put into this project - really big fan of squid and using it in one of my side projects.