From d39dcb9c5a8639ebd6396dc129beaed8db1dc7eb Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 4 Aug 2021 11:24:52 +0100
Subject: [PATCH] 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

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 pressure-vessel/wrap-setup.c | 134 +++++++++++++++++++++++++++++++++++
 pressure-vessel/wrap-setup.h |  25 +++++++
 pressure-vessel/wrap.c       | 117 ++++--------------------------
 3 files changed, 174 insertions(+), 102 deletions(-)

diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c
index b381bdbe8..eb734d07c 100644
--- a/pressure-vessel/wrap-setup.c
+++ b/pressure-vessel/wrap-setup.c
@@ -28,6 +28,7 @@
 
 #include "bwrap.h"
 #include "flatpak-run-private.h"
+#include "flatpak-utils-private.h"
 
 /*
  * Use code borrowed from Flatpak to share various bits of the
@@ -330,3 +331,136 @@ pv_wrap_move_into_scope (const char *steam_app_id)
   if (local_error != NULL)
     g_debug ("Cannot move into a systemd scope: %s", local_error->message);
 }
+
+/**
+ * pv_wrap_append_preload:
+ * @argv: (element-type filename): Array of command-line options to populate
+ * @variable: (type filename): Environment variable from which this
+ *  preload module was taken, either `LD_AUDIT` or `LD_PRELOAD`
+ * @option: (type filename): Command-line option to add to @argv,
+ *  either `--ld-audit` or `--ld-preload`
+ * @preload: (type filename): Path of preloadable module in current
+ *  namespace, possibly including special ld.so tokens such as `$LIB`,
+ *  or basename of a preloadable module to be found in the standard
+ *  library search path
+ * @flags: Flags to adjust behaviour
+ * @runtime: (nullable): Runtime to be used in container
+ * @exports: (nullable): Used to configure extra paths that need to be
+ *  exported into the container
+ *
+ * Adjust @preload to be valid for the container and append it
+ * to @argv.
+ */
+void
+pv_wrap_append_preload (GPtrArray *argv,
+                        const char *variable,
+                        const char *option,
+                        const char *preload,
+                        PvAppendPreloadFlags flags,
+                        PvRuntime *runtime,
+                        FlatpakExports *exports)
+{
+  g_return_if_fail (argv != NULL);
+  g_return_if_fail (option != NULL);
+  g_return_if_fail (preload != NULL);
+  g_return_if_fail (runtime == NULL || PV_IS_RUNTIME (runtime));
+
+  if (*preload == '\0')
+    return;
+
+  if (strstr (preload, "gtk3-nocsd") != NULL)
+    {
+      g_warning ("Disabling gtk3-nocsd %s: it is known to cause crashes.",
+                 variable);
+      return;
+    }
+
+  if ((flags & PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY)
+      && g_str_has_suffix (preload, "/gameoverlayrenderer.so"))
+    {
+      g_info ("Disabling Steam Overlay: %s", preload);
+      return;
+    }
+
+  /* A subsandbox will just have the same LD_PRELOAD as the
+   * Flatpak itself, except that we have to redirect /usr and /app
+   * into /run/parent. */
+  if (flags & PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX)
+    {
+      if (runtime != NULL
+          && (g_str_has_prefix (preload, "/usr/")
+              || g_str_has_prefix (preload, "/app/")
+              || g_str_has_prefix (preload, "/lib")))
+        {
+          g_autofree gchar *adjusted_path = NULL;
+
+          adjusted_path = g_build_filename ("/run/parent", preload, NULL);
+          g_debug ("%s -> %s", preload, adjusted_path);
+          g_ptr_array_add (argv,
+                           g_strdup_printf ("%s=%s",
+                                            option,
+                                            adjusted_path));
+        }
+      else
+        {
+          g_debug ("%s -> unmodified", preload);
+          g_ptr_array_add (argv,
+                           g_strdup_printf ("%s=%s",
+                                            option,
+                                            preload));
+        }
+
+      /* No FlatpakExports here: any file not in /usr or /app that
+       * is visible to our "parent" Flatpak app is also visible
+       * to us. */
+      return;
+    }
+
+  if (g_file_test (preload, G_FILE_TEST_EXISTS))
+    {
+      if (runtime != NULL
+          && (g_str_has_prefix (preload, "/usr/")
+              || g_str_has_prefix (preload, "/lib")))
+        {
+          g_autofree gchar *adjusted_path = NULL;
+
+          adjusted_path = g_build_filename ("/run/host", preload, NULL);
+          g_debug ("%s -> %s", preload, adjusted_path);
+          /* When using a runtime we can't write to /usr/ or /libQUAL/,
+           * so redirect this preloaded module to the corresponding
+           * location in /run/host. */
+          g_ptr_array_add (argv,
+                           g_strdup_printf ("%s=%s",
+                                            option,
+                                            adjusted_path));
+        }
+      else
+        {
+          const gchar *steam_path = NULL;
+          steam_path = g_getenv ("STEAM_COMPAT_CLIENT_INSTALL_PATH");
+
+          if (steam_path != NULL && flatpak_has_path_prefix (preload, steam_path))
+            {
+              g_debug ("Skipping exposing \"%s\" because it is located "
+                       "under the Steam client install path that we "
+                       "bind by default", preload);
+            }
+          else
+            {
+              g_debug ("%s -> unmodified, but added to exports", preload);
+              flatpak_exports_add_path_expose (exports,
+                                               FLATPAK_FILESYSTEM_MODE_READ_ONLY,
+                                               preload);
+            }
+
+          g_ptr_array_add (argv,
+                           g_strdup_printf ("%s=%s",
+                                            option,
+                                            preload));
+        }
+    }
+  else
+    {
+      g_info ("%s module '%s' does not exist", variable, preload);
+    }
+}
diff --git a/pressure-vessel/wrap-setup.h b/pressure-vessel/wrap-setup.h
index cfeef954a..45a7b463a 100644
--- a/pressure-vessel/wrap-setup.h
+++ b/pressure-vessel/wrap-setup.h
@@ -26,6 +26,7 @@
 
 #include "flatpak-bwrap-private.h"
 #include "flatpak-exports-private.h"
+#include "runtime.h"
 #include "wrap-pipewire.h"
 
 void pv_wrap_share_sockets (FlatpakBwrap *bwrap,
@@ -40,3 +41,27 @@ gboolean pv_wrap_use_host_os (FlatpakExports *exports,
 void pv_wrap_move_into_scope (const char *steam_app_id);
 
 const char *pv_wrap_get_steam_app_id (const char *from_command_line);
+
+/**
+ * PvAppendPreloadFlags:
+ * @PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX: The game will be run in
+ *  a Flatpak subsandbox
+ * @PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY: Disable the Steam Overlay
+ * @PV_APPEND_PRELOAD_FLAGS_NONE: None of the above
+ *
+ * Flags affecting the behaviour of pv_wrap_append_preload().
+ */
+typedef enum
+{
+  PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX = (1 << 0),
+  PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY = (1 << 1),
+  PV_APPEND_PRELOAD_FLAGS_NONE = 0
+} PvAppendPreloadFlags;
+
+void pv_wrap_append_preload (GPtrArray *argv,
+                             const char *variable,
+                             const char *option,
+                             const char *preload,
+                             PvAppendPreloadFlags flags,
+                             PvRuntime *runtime,
+                             FlatpakExports *exports);
diff --git a/pressure-vessel/wrap.c b/pressure-vessel/wrap.c
index 13acbb2a3..16e3ce41e 100644
--- a/pressure-vessel/wrap.c
+++ b/pressure-vessel/wrap.c
@@ -1320,6 +1320,7 @@ main (int argc,
   const char *steam_app_id;
   g_autoptr(GPtrArray) adverb_preload_argv = NULL;
   int result;
+  PvAppendPreloadFlags append_preload_flags = PV_APPEND_PRELOAD_FLAGS_NONE;
 
   setlocale (LC_ALL, "");
 
@@ -1763,6 +1764,10 @@ main (int argc,
       bwrap_filesystem_arguments = flatpak_bwrap_new (flatpak_bwrap_empty_env);
       exports = flatpak_exports_new ();
     }
+  else
+    {
+      append_preload_flags |= PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX;
+    }
 
   /* Invariant: we have bwrap or exports iff we also have the other */
   g_assert ((bwrap != NULL) == (exports != NULL));
@@ -2085,6 +2090,9 @@ main (int argc,
 
   adverb_preload_argv = g_ptr_array_new_with_free_func (g_free);
 
+  if (opt_remove_game_overlay)
+    append_preload_flags |= PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY;
+
   /* We need the LD_PRELOADs from Steam visible at the paths that were
    * used for them, which might be their physical rather than logical
    * locations. Steam doesn't generally use LD_AUDIT, but the Steam app
@@ -2106,108 +2114,13 @@ main (int argc,
 
       for (j = 0; j < len; j++)
         {
-          const char *preload = g_ptr_array_index (values, j);
-
-          g_assert (preload != NULL);
-
-          if (*preload == '\0')
-            continue;
-
-          if (strstr (preload, "gtk3-nocsd") != NULL)
-            {
-              g_warning ("Disabling gtk3-nocsd %s: it is known to cause crashes.",
-                         variable);
-              continue;
-            }
-
-          if (opt_remove_game_overlay
-              && g_str_has_suffix (preload, "/gameoverlayrenderer.so"))
-            {
-              g_info ("Disabling Steam Overlay: %s", preload);
-              continue;
-            }
-
-          /* A subsandbox will just have the same LD_PRELOAD as the
-           * Flatpak itself, except that we have to redirect /usr and /app
-           * into /run/parent. */
-          if (flatpak_subsandbox != NULL)
-            {
-              if (runtime != NULL
-                  && (g_str_has_prefix (preload, "/usr/")
-                      || g_str_has_prefix (preload, "/app/")
-                      || g_str_has_prefix (preload, "/lib")))
-                {
-                  g_autofree gchar *adjusted_path = NULL;
-
-                  adjusted_path = g_build_filename ("/run/parent", preload, NULL);
-                  g_debug ("%s -> %s", preload, adjusted_path);
-                  g_ptr_array_add (adverb_preload_argv,
-                                   g_strdup_printf ("%s=%s",
-                                                    option,
-                                                    adjusted_path));
-                }
-              else
-                {
-                  g_debug ("%s -> unmodified", preload);
-                  g_ptr_array_add (adverb_preload_argv,
-                                   g_strdup_printf ("%s=%s",
-                                                    option,
-                                                    preload));
-                }
-
-              /* No FlatpakExports here: any file not in /usr or /app that
-               * is visible to our "parent" Flatpak app is also visible
-               * to us. */
-              continue;
-            }
-
-          if (g_file_test (preload, G_FILE_TEST_EXISTS))
-            {
-              if (runtime != NULL
-                  && (g_str_has_prefix (preload, "/usr/")
-                      || g_str_has_prefix (preload, "/lib")))
-                {
-                  g_autofree gchar *adjusted_path = NULL;
-
-                  adjusted_path = g_build_filename ("/run/host", preload, NULL);
-                  g_debug ("%s -> %s", preload, adjusted_path);
-                  /* When using a runtime we can't write to /usr/ or /libQUAL/,
-                   * so redirect this preloaded module to the corresponding
-                   * location in /run/host. */
-                  g_ptr_array_add (adverb_preload_argv,
-                                   g_strdup_printf ("%s=%s",
-                                                    option,
-                                                    adjusted_path));
-                }
-              else
-                {
-                  const gchar *steam_path = NULL;
-                  steam_path = g_getenv ("STEAM_COMPAT_CLIENT_INSTALL_PATH");
-
-                  if (steam_path != NULL && flatpak_has_path_prefix (preload, steam_path))
-                    {
-                      g_debug ("Skipping exposing \"%s\" because it is located "
-                               "under the Steam client install path that we "
-                               "bind by default", preload);
-                    }
-                  else
-                    {
-                      g_debug ("%s -> unmodified, but added to exports", preload);
-                      flatpak_exports_add_path_expose (exports,
-                                                       FLATPAK_FILESYSTEM_MODE_READ_ONLY,
-                                                       preload);
-                    }
-
-                  g_ptr_array_add (adverb_preload_argv,
-                                   g_strdup_printf ("%s=%s",
-                                                    option,
-                                                    preload));
-                }
-            }
-          else
-            {
-              g_info ("%s module '%s' does not exist", variable, preload);
-            }
+          pv_wrap_append_preload (adverb_preload_argv,
+                                  variable,
+                                  option,
+                                  g_ptr_array_index (values, j),
+                                  append_preload_flags,
+                                  runtime,
+                                  exports);
         }
     }
 
-- 
GitLab