pressure-vessel: Remap preloadable modules better
Resolves: https://github.com/ValveSoftware/steam-runtime/issues/435
Requires !352 (merged).
-
pv_wrap_append_preload: Factor out
This reduces the size of wrap.c, which is something we should be careful to keep under control. It also allows us to unit-test this code.
Code changes made while factoring this out:
-
flatpak_subsandbox
andopt_remove_game_overlay
go via flags -
adverb_preload_argv
becomesargv
-
return
instead ofcontinue
since pv_wrap_append_preload() is only one loop iteration
-
-
pv_wrap_append_preload: Take environment as a parameter
This allows mock environment variables to be used during unit testing.
-
pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
An entry in LD_PRELOAD may be a bare SONAME like "libMangoHud.so", or a path containing special tokens like "/usr/$LIB/libMangoHud.so", or a literal path. Only literal paths are going to physically exist on disk.
For a path containing special tokens, we can still try to remap it, for example /usr/
LIB/libMangoHud.so. This is imperfect (if the host and container glibc disagree on the meaning of $LIB, then we won't load it correctly), but it's better than nothing. A subsequent commit will improve the handling of paths that contain special tokens.For a bare SONAME, we just pass it through as-is for now, as though it was a relative path. This is also imperfect, but better than nothing.
Another reason to want to make this change is that it allows us to unit-test this function: it's difficult to make assertions about the behaviour of a function that touches areas of the filesystem that are not under the control of the unit tests.
-
tests: Add test coverage for pv_wrap_append_preload
This has almost full branch coverage. We don't currently cover the case where STEAM_COMPAT_CLIENT_INSTALL_PATH is not set, but that's trivial anyway (we treat all paths as not matching it).
-
pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
PreloadModule is no longer used in pv-adverb, because we needed a different data structure there. Do a similar transformation here. This turns it inside out: instead of what's conceptually
map<variable=LD_AUDIT or LD_PRELOAD, values=array<string>>
we now have
array<struct<variable=LD_AUDIT or LD_PRELOAD, string>>
which is a bit easier to deal with in practice.
-
Add a function to classify strings to be loaded by libdl
(already Reviewed-by: @denittis, no need to review this again)
-
tests: Add coverage for non-literal LD_PRELOAD, LD_AUDIT modules
We need to remap these into something that can be loaded successfully in the container, but we don't yet.
-
pv_wrap_append_preload: Factor out most of it
Most of this code can be shared between the Flatpak and non-Flatpak code paths, avoiding internal duplication.
In future, we will also want to call into append_preload_internal() when we split up something like /usr/$LIB/mangohud/libMangoHud.so or /opt/plat-$PLATFORM/mypreload.so into a separate path per supported architecture: in such cases, we will still have to rewrite /usr into /run/host/usr, and add /opt/plat-*/mypreload.so to the FlatpakExports.
-
pv_wrap_append_preload: Remove a Flatpak/non-Flatpak branch
In reality, we have a FlatpakExports if and only if we are not creating a Flatpak subsandbox. If a caller somehow passes in a FlatpakExports while creating a subsandbox, it's harmless to tell it what to export.
-
pv_wrap_append_preload: Only consider paths for export if absolute
FlatpakExports isn't going to export relative paths anyway.
-
pv_wrap_append_preload: Classify loadable modules and behave accordingly
This doesn't make any practical difference yet, but it sets us up to be able to carry out finer-grained processing based on the loadable_flags.
-
pv_wrap_append_preload: Handle unsupported dynamic string tokens
We need to do this as highest-priority, since it invalidates the cleverer reasoning that we should be applying to LIB and PLATFORM.
-
tests: Assert that ${ORIGIN}, ${FUTURE} are handled correctly
-
pv_wrap_append_preload: Cope with ${LIB}, ${PLATFORM} in loadable modules
This partially resolves https://github.com/ValveSoftware/steam-runtime/issues/435, for the case where MangoHud was built with
-Dld_libdir_abs=true
, as in the upstreambuild.sh
or upstream binary releases. -
tests: Assert that modules with ${LIB} or ${PLATFORM} are handled
-
pv-unruntime: Split LD_PRELOAD on either colons or spaces
According to ld.so(8), either is equally valid for LD_PRELOAD (although note that LD_AUDIT only accepts colons).
-
pv-unruntime: Avoid passing --ld-audit= or --ld-preload= to pv-wrap
-
pv_wrap_append_preload: Handle libraries specified as a basename
This is a bit complicated, because there are two reasonable things that people might use LD_PRELOAD for, and in this mode it's particularly important to distinguish between them.
One is to inject arbitrary code, like MangoHud or fakeroot. In this case, we want to take the loadable module from the namespace in which the user initiated pv-wrap. We can do this the same way we deal with the ${LIB} and ${PLATFORM} dynamic string tokens: load it once per ABI, pass a separate option to pv-adverb for each one, and let pv-adverb recombine them.
The other is to work around libraries not being loaded soon enough, like the way people sometimes use LD_PRELOAD="libpthread.so.0 libGL.so.1" to force an optirun library to be loaded. In this case, we absolutely do not want to import the host library of that name into the container unconditionally, because if we do, it will sabotage our careful efforts to get the correct instance of libpthread.so.0 to be chosen. In this case, assume that the user meant "take whatever libpthread.so.0 you would naturally load, and preload it into each executable".
-
tests: Test that LD_PRELOAD=libbasename.so is handled as expected
Merge request reports
Activity
Break up the commit that adds the tests: some of it is preparation adding more design-for-test in FlatpakExports (which should go upstream to Flatpak, ideally accompanied by test coverage that makes use of it)
@denittis Please could you take a look at https://github.com/flatpak/flatpak/pull/4352 for that?
added 34 commits
- c5aa8b72 - tests: Avoid wrapping tests with prove(1)
- e23ad1e0 - tests: If GLib doesn't support TAP, enable debug messages instead
- baac0995 - sysroot: Use gcc-4.8, g++-4.8 for native x86_64 build too
- 6a7f5b5b - tests: Link pressure-vessel tests to static libsteam-runtime-tools
- 0153dd4c - exports: Fix shadowed local variable and remove suppression
- 7ac4c16d - exports: Expose /var/usrlocal if "--filesystem=host" is specified
- 124134dc - exports: Update last-update version
- 23121a38 - exports: Sync with flatpak/flatpak#4352
- 1ae4ccdd - pressure-vessel: Only have one list of modules per PvPreloadList
- a5a1a81a - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- 93f19cfa - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- 17a3eee7 - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- 3d83fd0f - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- 33bc7fdd - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- b84e49a8 - pv_preload_lists_adjust_to_container: Factor out
- b569fe55 - build: Separate most of pv-wrap into a convenience library
- 7024de04 - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 925740ba - tests: Add test coverage for pv_wrap_append_preload
- 93ad68ad - adverb.1: Remove blank line from arguments
- ea68a993 - pv-adverb: Index abi_tuples_details with abi, not jj
- e14d02df - PvRuntime: Factor out the list of supported architectures
- 05b99767 - pv-adverb: Use pv_multiarch_details
- 73c35c86 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- 852346a9 - pv-adverb: Use a small fixed array instead of a hash table
- cb6c2050 - pv-adverb: Factor out ptr_array_add_unique
- 1610388d - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- 54a0a4df - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 77480685 - pv-utils: Remove PreloadModule data structure
- 039eb0c7 - pv-adverb: Add an ABI to each PreloadModule
- 97e5fb8e - pv-adverb: Allow setting the architecture of a module in the CLI
- 646a873b - Add a function to classify strings to be loaded by libdl
- aee54755 - _srt_gstring_replace: Add
- fe74ee9c - capture-libs: Add if-not-in-container flag
- 493aa853 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit listadded 41 commits
-
493aa853...50fd8483 - 10 commits from branch
master
- eb003efc - tests: Link pressure-vessel tests to static libsteam-runtime-tools
- 5cc6e025 - exports: Fix shadowed local variable and remove suppression
- 8f3e4237 - exports: Expose /var/usrlocal if "--filesystem=host" is specified
- dcec0b60 - exports: Update last-update version
- 9896d08c - exports: Sync with flatpak/flatpak#4352
- 4235da0e - pressure-vessel: Only have one list of modules per PvPreloadList
- 3d4342b6 - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- 96cc9fa1 - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- db029a2a - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- e15f42f2 - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- 1f527e50 - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- cd7601db - pv_preload_lists_adjust_to_container: Factor out
- e6a47f1a - build: Separate most of pv-wrap into a convenience library
- c5048e53 - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 586756b2 - tests: Add test coverage for pv_wrap_append_preload
- ae10eb22 - adverb.1: Remove blank line from arguments
- 6b93938c - pv-adverb: Index abi_tuples_details with abi, not jj
- 1406786c - PvRuntime: Factor out the list of supported architectures
- 588b1272 - pv-adverb: Use pv_multiarch_details
- ea8b33bb - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- 4542a2be - pv-adverb: Use a small fixed array instead of a hash table
- 352779d0 - pv-adverb: Factor out ptr_array_add_unique
- 7a63a79c - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- 9b89e4e7 - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 5fbf4ede - pv-utils: Remove PreloadModule data structure
- 9f6e2037 - pv-adverb: Add an ABI to each PreloadModule
- 5ea63bab - pv-adverb: Allow setting the architecture of a module in the CLI
- 76459430 - Add a function to classify strings to be loaded by libdl
- 454258f4 - _srt_gstring_replace: Add
- 1afa455f - capture-libs: Add if-not-in-container flag
- 4d31798b - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit list-
493aa853...50fd8483 - 10 commits from branch
added 27 commits
- 8abb39dd - pressure-vessel: Only have one list of modules per PvPreloadList
- 51c81a91 - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- d326a99e - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- 173179fb - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- fa5d8ea1 - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- b424d2e7 - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- 1c86a85c - pv_preload_lists_adjust_to_container: Factor out
- 2d06747f - build: Separate most of pv-wrap into a convenience library
- 7bb386dd - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 62d5b2db - tests: Add test coverage for pv_wrap_append_preload
- 15e00b03 - adverb.1: Remove blank line from arguments
- 100ff559 - pv-adverb: Index abi_tuples_details with abi, not jj
- ebbbd712 - PvRuntime: Factor out the list of supported architectures
- 8d2d5f16 - pv-adverb: Use pv_multiarch_details
- 9a7294a3 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- 34d8808f - tests: Add test coverage for passing incorrect options to pv-adverb
- 828cc9e4 - pv-adverb: Use a small fixed array instead of a hash table
- 0c35eace - pv-adverb: Factor out ptr_array_add_unique
- 4cb70ff6 - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- f908a562 - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 39cd47dc - pv-utils: Remove PreloadModule data structure
- a718b15d - pv-adverb: Add an ABI to each PreloadModule
- f017d24e - pv-adverb: Allow setting the architecture of a module in the CLI
- b02851ad - Add a function to classify strings to be loaded by libdl
- 76536b12 - _srt_gstring_replace: Add
- 49c0362e - capture-libs: Add if-not-in-container flag
- 8f63abf5 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit listadded 32 commits
-
ff67648d - 1 commit from branch
master
- 0dc3a3fe - exports: Fix shadowed local variable and remove suppression
- df0eafd5 - exports: Expose /var/usrlocal if "--filesystem=host" is specified
- 816257b6 - exports: Update last-update version
- 8d19882f - exports: Sync with flatpak/flatpak#4352
- 7bdca163 - pressure-vessel: Only have one list of modules per PvPreloadList
- 548a82a4 - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- 5058b4d6 - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- b2855c5b - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- 07a5d50f - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- 13bcd2c6 - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- c0886247 - pv_preload_lists_adjust_to_container: Factor out
- 73223182 - build: Separate most of pv-wrap into a convenience library
- da905aea - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- c354c5a8 - tests: Add test coverage for pv_wrap_append_preload
- 6e876b8d - adverb.1: Remove blank line from arguments
- 3df647a2 - pv-adverb: Index abi_tuples_details with abi, not jj
- 7a4a8324 - PvRuntime: Factor out the list of supported architectures
- af4f3e66 - pv-adverb: Use pv_multiarch_details
- 56530402 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- df550750 - tests: Add test coverage for passing incorrect options to pv-adverb
- ccb16626 - pv-adverb: Use a small fixed array instead of a hash table
- be82f042 - pv-adverb: Factor out ptr_array_add_unique
- c75d490a - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- bf25c341 - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 1dd011b2 - pv-utils: Remove PreloadModule data structure
- 491d7944 - pv-adverb: Add an ABI to each PreloadModule
- c6931328 - pv-adverb: Allow setting the architecture of a module in the CLI
- cef072f9 - Add a function to classify strings to be loaded by libdl
- a2dfd596 - _srt_gstring_replace: Add
- 98f2e7cc - capture-libs: Add if-not-in-container flag
- b71a99e8 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit list-
ff67648d - 1 commit from branch
added 26 commits
- a475d7d3 - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- f493344b - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- bcd209bc - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- 6836c043 - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- a5789452 - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- bafd6cbb - pv_preload_lists_adjust_to_container: Factor out
- 22e78261 - build: Separate most of pv-wrap into a convenience library
- 81fdf846 - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 9722d2e2 - tests: Add test coverage for pv_wrap_append_preload
- 79a888e2 - adverb.1: Remove blank line from arguments
- d6477252 - pv-adverb: Index abi_tuples_details with abi, not jj
- 1b68b7e9 - PvRuntime: Factor out the list of supported architectures
- 87d98117 - pv-adverb: Use pv_multiarch_details
- deb4fb09 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- 74a41c4a - tests: Add test coverage for passing incorrect options to pv-adverb
- e92a85ff - pv-adverb: Use a small fixed array instead of a hash table
- 3640cc09 - pv-adverb: Factor out ptr_array_add_unique
- c5c14f93 - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- 26007320 - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 251257b7 - pv-utils: Remove PreloadModule data structure
- ec0f02a7 - pv-adverb: Add an ABI to each PreloadModule
- 753906e0 - pv-adverb: Allow setting the architecture of a module in the CLI
- 556e2637 - Add a function to classify strings to be loaded by libdl
- c1170ebc - _srt_gstring_replace: Add
- b713d6a8 - capture-libs: Add if-not-in-container flag
- 3c085670 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit listadded 21 commits
- fd32bf7f - pv_wrap_append_preload: Factor out
- 3b8fdce8 - build: Separate most of pv-wrap into a convenience library
- 9a22851b - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 9e91c01e - tests: Add test coverage for pv_wrap_append_preload
- 15f4584c - adverb.1: Remove blank line from arguments
- 7369c494 - pv-adverb: Index abi_tuples_details with abi, not jj
- 3ad6065c - PvRuntime: Factor out the list of supported architectures
- b4bef0d4 - pv-adverb: Use pv_multiarch_details
- 4938b1bd - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- e9401a8c - tests: Add test coverage for passing incorrect options to pv-adverb
- 765fee44 - pv-adverb: Use a small fixed array instead of a hash table
- 71badd54 - pv-adverb: Factor out ptr_array_add_unique
- b1599bce - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- d093356a - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 4525898a - pv-utils: Remove PreloadModule data structure
- 180c9b1e - pv-adverb: Add an ABI to each PreloadModule
- dc532469 - pv-adverb: Allow setting the architecture of a module in the CLI
- e5f893c9 - Add a function to classify strings to be loaded by libdl
- 85144ca6 - _srt_gstring_replace: Add
- 592bfb31 - capture-libs: Add if-not-in-container flag
- a65201a5 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit listrequested review from @denittis
assigned to @smcv
- Resolved by Simon McVittie
- Resolved by Simon McVittie
- Resolved by Simon McVittie
- Resolved by Simon McVittie
There is a lot going on in this MR!
However apart from the few small comments that I left, this looks good to me.
mentioned in merge request !343 (merged)
added 28 commits
-
a65201a5...779633ba - 2 commits from branch
master
- 61536ed3 - exports: Sync with flatpak/flatpak#4352
- 3c94073b - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- a79bfdc3 - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- 3777cc91 - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- f3803f74 - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- 5c9709ec - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- 25962507 - pv_wrap_append_preload: Factor out
- 117baa23 - build: Separate most of pv-wrap into a convenience library
- 65b69521 - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 1df0a7d3 - tests: Add test coverage for pv_wrap_append_preload
- 9cc72398 - adverb.1: Remove blank line from arguments
- d5dcc69a - pv-adverb: Index abi_tuples_details with abi, not jj
- d5521655 - PvRuntime: Factor out the list of supported architectures
- d6518d91 - pv-adverb: Use pv_multiarch_details
- 74f99167 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- d727de0d - tests: Add test coverage for passing incorrect options to pv-adverb
- b79d2802 - pv-adverb: Use a small fixed array instead of a hash table
- ff84febf - pv-adverb: Factor out ptr_array_add_unique
- 18096297 - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- d8f6231d - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- c86a7efe - pv-utils: Remove PreloadModule data structure
- 1e07a251 - pv-adverb: Add an ABI to each PreloadModule
- de8a8774 - pv-adverb: Allow setting the architecture of a module in the CLI
- 8614eec4 - Add a function to classify strings to be loaded by libdl
- 856e81ea - capture-libs: Add if-not-in-container flag
- a1497858 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit list-
a65201a5...779633ba - 2 commits from branch
added 27 commits
-
ff998a3d - 1 commit from branch
master
- 7465b423 - exports: Sync with flatpak/flatpak#4352
- 0c0dd64f - pressure-vessel: Exclude adjusted LD_PRELOAD from PreloadModule[]
- d6e34410 - pv-wrap: Hoist ld.so.blockedlist adjustment out of loop
- 111dcaeb - pv-wrap: Generalize SHARED_LIBRARY_GUARD_CONFIG handling
- 31361808 - pv-wrap: Don't try to export non-absolute LD_PRELOAD entries
- ddefd551 - pv-wrap: Try to map LD_PRELOAD modules into container even if nonexistent
- f6eddb14 - pv_wrap_append_preload: Factor out
- d6aaa951 - build: Separate most of pv-wrap into a convenience library
- c92142e5 - tests: Link pressure-vessel tests to most of pressure-vessel-wrap
- 879dd0a3 - tests: Add test coverage for pv_wrap_append_preload
- b8df9ae7 - adverb.1: Remove blank line from arguments
- 32a46ded - pv-adverb: Index abi_tuples_details with abi, not jj
- 15f71b46 - PvRuntime: Factor out the list of supported architectures
- a22edae6 - pv-adverb: Use pv_multiarch_details
- e15a7996 - tests: Add test coverage for pv-adverb LD_AUDIT, LD_PRELOAD handling
- 0942b39e - tests: Add test coverage for passing incorrect options to pv-adverb
- ff9e2af5 - pv-adverb: Use a small fixed array instead of a hash table
- 8e5ae3cb - pv-adverb: Factor out ptr_array_add_unique
- 56fd1969 - pv-adverb: Put LD_AUDIT, LD_PRELOAD modules in an array of structs
- 900527be - pv-wrap: Replace PreloadModule[] with a GArray of WrapModule
- 452017ed - pv-utils: Remove PreloadModule data structure
- 96dadffc - pv-adverb: Add an ABI to each PreloadModule
- 864df9c0 - pv-adverb: Allow setting the architecture of a module in the CLI
- 7531bb0c - Add a function to classify strings to be loaded by libdl
- 4f03bd78 - capture-libs: Add if-not-in-container flag
- 01f28422 - pv-wrap: Improve support for non-literal LD_PRELOAD modules
Toggle commit list-
ff998a3d - 1 commit from branch
mentioned in merge request !346 (merged)