Skip to content

pressure-vessel: Remap preloadable modules better

Simon McVittie requested to merge wip/t29490 into master

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 and opt_remove_game_overlay go via flags
    • adverb_preload_argv becomes argv
    • return instead of continue 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 to /run/host/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 upstream build.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

Edited by Simon McVittie

Merge request reports