ValveSoftware / steam-runtime

A runtime environment for Steam applications
Other
1.17k stars 86 forks source link

Fails to launch if stack size rlimit is less than the default 8 MiB #653

Closed smcv closed 4 months ago

smcv commented 4 months ago

Your system information

Steps for reproducing this issue:

Launch Steam with a RLIMIT_STACK (ulimit -s, stack size limit) significantly less than the default 8 MiB.

An easy way to reproduce this without changing global system configuration is prlimit -s5123456 steam. The precise limit doesn't matter: this one was chosen to make it easy to count the number of digits.

Expected result

On https://github.com/ValveSoftware/steam-for-linux/issues/10549#issuecomment-1975603939, @wancoder expected Steam to run successfully with a stack size limit of 4 MiB. I don't know whether this should be considered to be a reasonable expectation or not.

Actual result

x86_64-linux-gnu-capsule-capture-libs, an internal part of the container runtime framework, crashes with signal 11 (segmentation fault) when it runs out of stack space.

Workaround

Run Steam with a stack size limit of at least 8 MiB, for example ulimit -s8192 in bash.

8 MiB has been the Linux default since version 1.3.7 in July 1995.

smcv commented 4 months ago

Analysis

I am not aware of any performance or security advantage to setting a low limit for the stack size. The only reason to limit the stack size for the main thread is that if a program bug results in infinite recursion, a limited stack size will cause the program to terminate with SIGSEGV instead of continuing to run until physical memory is exhausted: this means that a stack size that was considered reasonable in 1995 will be either reasonable or too small today.

Linux is a Unix-inspired system that makes extensive use of virtual memory, so allocating a large amount of virtual memory does not actually consume additional physical memory. x86_64-linux-gnu-capsule-capture-libs is not a long-running process (it carries out its task and then exits), so even if it contained major memory leaks, the allocated memory would be cleaned up within a matter of seconds.

While setting up the container, x86_64-linux-gnu-capsule-capture-libs needs to do relatively low-level things with shared libraries, so it is written in a style that defends against unintended interactions between dlmopen() namespaces by avoiding heap allocations and preferring to use PATH_MAX-sized arrays on the stack. This means that it can use up stack space more quickly than is typically seen.

The most likely solution for this would be for either Steam or the container runtime to increase the stack size limit to at least its 1995 default if it is less than that on entry.

wancoder commented 4 months ago

@smcv Practicality is under two use cases:

Normally not an issue for standard installations. So this serves as a reminder to anyone who, changed nothing and have touched their stack limits for single/multi user environments to check it with ulimit -a. If they are running a custom sandboxing environment it could be a factor through a different security setting.

wancoder commented 4 months ago

One work around to support a smaller stack size is to simply break up the iterations into chunks, isolated generators.

smcv commented 4 months ago

Reducing the stack size limit does not reduce memory overhead: each process only uses as much stack as it finds that it needs to use. The limit is a limit, not an amount that will be preallocated.

Reducing the stack size limit also does not prevent processes from allocating an unlimited amount of heap memory. If this particular program needed to allocate less stack memory, the way it would achieve that would be to put the same strings on the heap instead (using the same amount of memory, but with a higher probability of it being leaked). This does not seem like a win.

Forcing development of more efficient logic by constraining resources

You are welcome to do this for your own software if you feel that it's important, but please don't assume that the whole world has the same priorities you do.

wancoder commented 4 months ago

You are welcome to do this for your own software if you feel that it's important, but please don't assume that the whole world has the same priorities you do.

I got no time to care about any of that, so reading you write that is a bit of a shock:

A dude only wants to:

In the past I checked the savings,maybe the preemptive memory allocation algorithm has improved since then. It was the difference between 32MB and 128MB out of 512MB on my embedded system back then, this is why I myself use it from time to time but forgot to switch it back. It is possible somebody else does the same, why not drop a reminder since it tripped me up. I changed my ulimit setting ages ago.

smcv commented 4 months ago

pressure-vessel 0.20240306.0, as included in Steam client update 1709846872, uses heap allocation for this.