From ba0ffa1ec2fd183933db90b87b26a5865c1d1f2f Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 7 Feb 2025 13:48:27 +0000
Subject: [PATCH] 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

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 pressure-vessel/wrap-setup.c | 67 ++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c
index 08c710134..edda14e40 100644
--- a/pressure-vessel/wrap-setup.c
+++ b/pressure-vessel/wrap-setup.c
@@ -545,6 +545,63 @@ pv_wrap_move_into_scope (const char *steam_app_id)
     g_debug ("Cannot move into a systemd scope: %s", local_error->message);
 }
 
+/*
+ * export_path_for_preload:
+ * @context: Context we are running in
+ * @path: Absolute path to a LD_PRELOAD or LD_AUDIT module
+ *
+ * Return the path that we would need to add to `context->exports`
+ * to make this module available to the container, or %NULL if no path
+ * should be added.
+ *
+ * Returns: (transfer full) (nullable): Path to export, or %NULL
+ */
+static gchar *
+export_path_for_preload (PvWrapContext *context,
+                         const char *path)
+{
+  g_autofree gchar *parent = NULL;
+  glnx_autofd int parent_fd = -1;
+  FlatpakExports *exports = context->exports;
+
+  if (exports == NULL)
+    return NULL;
+
+  /* Normally we export the directory containing the LD_PRELOAD module,
+   * and not the module itself. For example, if we're going to load
+   * /home/me/.local/lib64/mangohud/libMangoHud_shim.so,
+   * we want to share /home/me/.local/lib64/mangohud with the
+   * container, so that libMangoHud_shim.so can dlopen
+   * libMangoHud_opengl.so in the same directory, as expected.
+   * This is a compromise between exporting too much (which we
+   * could easily do if we walked further up the directory tree,
+   * for example undoing the effect of --unshare-home)
+   * and exporting too little (just libMangoHud_shim.so, which
+   * doesn't do anything useful on its own). */
+  parent = g_path_get_dirname (path);
+  parent_fd = _srt_sysroot_open (context->current_root,
+                                 parent,
+                                 SRT_RESOLVE_FLAGS_MUST_BE_DIRECTORY,
+                                 NULL,
+                                 NULL);
+
+  /* Special case: we don't want to export all of $HOME if we are doing
+   * --unshare-home, so override that to only exporting the minimum
+   *  that we can: the module itself. */
+  if (context->current_home_fd >= 0
+      && parent_fd >= 0
+      && _srt_fstatat_is_same_file (context->current_home_fd, "",
+                                    parent_fd, ""))
+    {
+      g_debug ("Not exporting parent of \"%s\" because it is $HOME",
+               path);
+      return g_strdup (path);
+    }
+
+  g_debug ("Exporting parent of \"%s\" -> \"%s\"", path, parent);
+  return g_steal_pointer (&parent);
+}
+
 static void
 append_preload_internal (PvWrapContext *context,
                          GPtrArray *argv,
@@ -765,13 +822,15 @@ append_preload_per_architecture (PvWrapContext *context,
 
       if (path != NULL)
         {
+          g_autofree gchar *export_path = export_path_for_preload (context, path);
+
           g_debug ("Found %s version of %s at %s",
                    multiarch_tuple, preload, path);
           append_preload_internal (context,
                                    argv,
                                    which,
                                    i,
-                                   path,
+                                   export_path,
                                    path,
                                    flags);
         }
@@ -824,7 +883,7 @@ append_preload_basename (PvWrapContext *context,
                                argv,
                                which,
                                PV_UNSPECIFIED_ABI,
-                               NULL,
+                               NULL,  /* don't export anything */
                                preload,
                                flags);
     }
@@ -929,6 +988,8 @@ pv_wrap_append_preload (PvWrapContext *context,
           }
         else
           {
+            g_autofree gchar *export_path = export_path_for_preload (context, preload);
+
             /* All dynamic tokens should be handled above, so we can
              * assume that preload is a concrete filename */
             g_warn_if_fail ((loadable_flags & SRT_LOADABLE_FLAGS_DYNAMIC_TOKENS) == 0);
@@ -936,7 +997,7 @@ pv_wrap_append_preload (PvWrapContext *context,
                                      argv,
                                      which,
                                      PV_UNSPECIFIED_ABI,
-                                     preload,
+                                     export_path,
                                      preload,
                                      flags);
           }
-- 
GitLab