overlay: remove shell spawns in init_system_info() to avoid pressure-vessel seccomp failures - #1983
Conversation
We don't know what component is applying these seccomp filters: it might be something "larger" than the Steam Linux Runtime / pressure-vessel container, or it might be something that runs inside the container, like Proton. SLR/PV itself does not add any seccomp rules. |
Yes, I agree, my statement wasn't precise here. But whatever the root cause of the observed crashes is, it uncovered problematic code paths in MangoHud which should be fixed no matter what happens elsewhere. I deliberately chose to not write "sandbox" here to support the fact that the pressure-vessel container isn't a security boundary. We don't know yet where the seccomp filter originates but MangoHud trips over it. These commits mitigate the crashes (but there are still other potential crash scenarios, with or without pressure-vessel / SRT). |
When `glxinfo` is not called, setting up `MANGOHUD_RECURSION` is dead code: we read, set and unset the variable without any effect. Dropping this block also avoids unnecessary environment mutation in-process. `getenv()`/`setenv()` are not thread-safe in general, so avoiding them here reduces risk in multi-threaded contexts. v2: Dropping this code completely instead of only disabling it as per the discussion with @flightlessmango. Ref: flightlessmango#1983 (comment)
Spawning a shell from inside pressure-vessel is fragile and can fail due to seccomp constraints, causing noisy logs and host coredumps. Replace the shell pipeline (`sh|sed|tail`) used in `init_system_info()` with native parsing and Linux APIs where available. This avoids subprocess creation entirely for the covered paths. This does not address every shell-based code path yet (for example the disabled OpenGL version reader could be migrated similarly), but that is outside the scope of this change. The primary goal is to prevent `/usr/bin/dash` crashes from the Steam Runtime when MangoHud runs inside pressure-vessel. Ref: ValveSoftware/steam-runtime#804
A previous commit removed subprocess spawners from `init_system_info()`. This allows us to reduce `getenv()`/`setenv()` usage to the absolute minimum and lower risk in multi-threaded contexts. The environment workaround is now only applied around the remaining subprocess call used to query the Wine version. Also add a preprocessor error in the disabled OpenGL callout block so the `LD_PRELOAD` workaround is not missed if that callout is re-enabled.
8ae04a8 to
64dbedd
Compare
|
@flightlessmango Before I invest more time in this potentially to be removed code, is there a branch with your current refactoring progress, or are there plans when it is ready? |
|
I just saw that |
|
The working branch is server2. It's a structural change shifting MangoHud to a server/client model, so the main areas to look at are the |
|
Thanks, I'll look at it. I think one focus area is that the Vulkan layer avoids mutating states it doesn't control, especially if thread safety is concerned (getenv/setenv), and avoid forking shell pipelines. It looks like a client/server model is a very good way to do that. I'm somewhat excited. :-) |
|
With latest Proton Experimental (v11), I now see these: This is caused by the shell spawns which my patches didn't touch yet, notably spawning wine to read the wine version. |
3 participants
This PR removes shell-based system info collection from
init_system_info()and replaces it with native code paths.Motivation
In Steam Runtime / pressure-vessel setups (notably via Wine + Vulkan layer), spawning
/bin/shpipelines from MangoHud can fail under runtime/seccomp constraints and produce noisy logs and host coredumps (e.g.dash).By removing those shell callouts, MangoHud avoids this failure mode during initialization.
Ref: ValveSoftware/steam-runtime#804
Exact scope of this PR
init_system_info()no longer shells out for basic system infoReplaced shell pipelines with native logic:
ram:sedon/proc/meminfosysinfo(2)(totalram * mem_unit, converted to KiB)kernel:uname -rshell calluname(2)viautsname.releasecpu:sed ... /proc/cpuinfo | sed 's/([^)]*)//g;s/ / /g' | tail -n1/proc/cpuinfoparsing in C++, plus equivalent cleanup of parenthesized blocks/spacingos:sedon/etc/os-releaseforPRETTY_NAME/etc/os-releaseparsing in C++, then quote removalEnvironment mutation scope reduced
Because the above shell spawns are gone,
LD_PRELOADhandling was narrowed down to only where subprocess spawning is still needed (Wine version probing path).The disabled OpenGL/glxinfo callout block is guarded so re-enabling it cannot silently miss the required
LD_PRELOADworkaround.No broader refactors
This PR intentionally does not migrate every remaining external callout in the file. Only the
init_system_info()shell pipelines are targeted to fix the runtime failure mode above.Behavioral notes
/usr/bin/dashcoredumps in affected pressure-vessel scenarios.Future considerations
This change reduces
getenv()/setenv()usage in one hot path, but the broader pattern still exists in other parts of the codebase.Given MangoHud runs as a Vulkan layer inside the game process, process-global environment mutation is risky:
getenv()/setenv()are process-global and generally not a good fit for highly concurrent in-process code.Follow-up work could: