Skip to content
Snippets Groups Projects

Export parent directory of LD_PRELOAD modules

Merged Simon McVittie requested to merge wip/task595-export-parent into main

Another step towards solving steamrt/tasks#595, based on the refactoring in !783 (merged) and !784 (merged).

!780 should be rebased onto this, but I'm not there yet.

/cc @refi64 @denittis


  • pv-wrap: Have a file descriptor for $HOME in the PvWrapContext

    This will allow the LD_PRELOAD handling code to use a real or mock home directory, as appropriate.

  • wrap-setup test: Assert that paths passed to fixture_populate_dir are relative

  • wrap-setup test: Use a temporary variable for the path to create

    No functional change.

  • wrap-setup test: Convert absolute paths to relative

    This will make it clearer when writing tests where a LD_PRELOAD entry is very similar (or even identical) to the path that we need to create to represent it.

  • wrap-setup test: Move list of LD_PRELOAD entries to top level

    This will let us make the tests that involve it more declarative, by grouping the various inputs and expectations related to one LD_PRELOAD entry into the struct associated with that entry.

  • wrap-setup test: Make setup of mock sysroot more declarative

  • wrap-setup test: Move expectations into same data structure as inputs

  • wrap-setup test: Stop asserting that "/" is marked as exported

    This makes no sense: the comment above indicates that we didn't want this path to be exported, and if it was exported then we would be unable to have any control over the filesystem namespace.

    In fact, it is not exported: the fact that it appears to be exported is an implementation quirk of flatpak_exports_path_get_mode(), which returns FLATPAK_FILESYSTEM_MODE_READ_WRITE because none of the zero remaining path segments is unmapped.

  • wrap-setup test: Store a pointer to the configuration in the fixture

  • wrap-setup test: Set a mock home directory

    Otherwise, we can't test behaviours that depend on the home directory.

  • wrap-setup test: Make assertions about the FlatpakExports declarative

    This keeps the inputs and their expected results together.

  • wrap-setup test: Clarify why it's correct to not export $HOME

    steamrt/tasks#595

  • wrap-setup test: Systematically check ancestors of LD_PRELOADs

    In two cases we were asserting that the grandparent directory is not exported, and the LD_PRELOAD module itself is, but making no assertion about the parent directory of the LD_PRELOAD module.

    To make modules like MangoHud work more reliably, in fact we want to export the parent directory in most cases.

    steamrt/tasks#595

  • wrap-setup test: Test export behaviour in a subdirectory of $HOME

    Contrast this with the previous test coverage for a module directly inside $HOME: in that situation we don't want to overrule --unshare-home, but in this situation we do want to share a small subset of it.

    steamrt/tasks#595

  • pv-wrap: Export parent dir of LD_PRELOAD modules, unless it's $HOME

    LD_PRELOAD modules are an inherently fragile mechanism, and any symbols exported by either a LD_PRELOAD module or its direct dependencies will "interpose" in front of symbols of the same name from any other library. In some cases this can lead to crashes, for example when the Python ctypes module from Steam Runtime 2 'soldier' (which depends on libffi.so.6) gets its libffi references resolved to symbols from a more modern host system's libffi.so.7 or libffi.so.8 pulled in by the dependency chain:

      libMangoHud.so -> libwayland-client.so.0 -> libffi.so.8

    Authors of LD_PRELOAD modules like MangoHud can avoid this situation by making their LD_PRELOAD module only depend on highly long-term-stable projects (ideally only glibc), and then loading the implementation of its real functionality via dlopen(), using RTLD_LOCAL|RTLD_DEEPBIND to avoid symbol conflicts.

    For example, newer versions of MangoHud separate the module into libMangoHud_shim.so (the actual LD_PRELOAD module, no dependencies except glibc) and libMangoHud_opengl.so (dlopen'd by the shim module, depends on whatever it needs to depend on).

    This is a good approach and we should encourage it, but in the current pressure-vessel codebase it isn't going to be completely reliable for modules that are installed other than in /usr: for libMangoHud_shim.so to be able to load libMangoHud_opengl.so, we need to ensure that they are both visible in the filesystem of the game's container. However, since we can't know what modules and/or libraries libMangoHud_shim.so is going to load at runtime, we will have to use a heuristic.

    The heuristic I've chosen here is to say that if you have LD_PRELOAD=/path/to/some/module.so, then instead of sharing /path/to/some/module.so with the container as we did previously, we should share the parent directory /path/to/some/. In particular, this is enough for MangoHud's requirements.

    However, one notable exception is that if the module is situated directly inside $HOME, and we are using --unshare-home to avoid sharing the home directory with the container, we probably do not want to overrule that. In this case, users will still need to use PRESSURE_VESSEL_FILESYSTEMS_RO to share the dependency explicitly.

    steamrt/tasks#595

  • wrap-setup test: Assert we export parents of LD_PRELOADs if appropriate

    steamrt/tasks#595

Edited by Simon McVittie

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Simon McVittie marked this merge request as draft

    marked this merge request as draft

  • Simon McVittie removed review request for @denittis

    removed review request for @denittis

  • Simon McVittie added 6 commits

    added 6 commits

    • e590fbbf...44855f60 - 3 commits from branch main
    • 9e257a57 - pv-wrap: Have a file descriptor for $HOME in the PvWrapContext
    • 0fd0ec78 - pv-wrap: Export parent dir of LD_PRELOAD modules, unless it's $HOME
    • 043feeff - tests: Check for intended export behaviour

    Compare with previous version

  • Simon McVittie added 15 commits

    added 15 commits

    • 043feeff...f4bae94a - 5 earlier commits
    • 8ba3b8d6 - wrap-setup test: Move expectations into same data structure as inputs
    • 76995e52 - wrap-setup test: Stop asserting that "/" is marked as exported
    • 00073507 - wrap-setup test: Store a pointer to the configuration in the fixture
    • 065cffd8 - wrap-setup test: Set a mock home directory
    • be40344c - wrap-setup test: Make assertions about the FlatpakExports declarative
    • 48f92a03 - wrap-setup test: Clarify why it's correct to not export $HOME
    • b69911a7 - wrap-setup test: Systematically check ancestors of LD_PRELOADs
    • 44a94426 - wrap-setup test: Test export behaviour in a subdirectory of $HOME
    • ba0ffa1e - pv-wrap: Export parent dir of LD_PRELOAD modules, unless it's $HOME
    • 80260f1c - wrap-setup test: Assert we export parents of LD_PRELOADs if appropriate

    Compare with previous version

  • added 1 commit

    • 9ca47131 - wrap-setup test: Assert we export parents of LD_PRELOADs if appropriate

    Compare with previous version

  • Simon McVittie marked this merge request as ready

    marked this merge request as ready

  • Simon McVittie changed the description

    changed the description

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading