run: Add option to share /dev/shm between instances of an app-ID

Similar to /tmp, applications might well use /dev/shm as an IPC
rendezvous between instances, which wouldn't have worked without
--device=shm until now.

Because /dev/shm has specific characteristics (in particular it's
meant to always be a tmpfs), we offload the actual storage into a
subdirectory of the real /dev/shm. Because /dev/shm is a shared
directory between all uids, we have to be extra-careful how we
do this, which is why the test coverage here is important.

This is done on an opt-in basis because of its extra complexity.

Signed-off-by: Simon McVittie <smcv@collabora.com>

.github/workflows/check.yml

@@ -41,6 +41,8 @@ jobs:
         libjson-glib-dev shared-mime-info desktop-file-utils libpolkit-agent-1-dev libpolkit-gobject-1-dev \
         libseccomp-dev libsoup2.4-dev libsystemd-dev libxml2-utils libgpgme11-dev gobject-introspection \
         libgirepository1.0-dev libappstream-glib-dev libdconf-dev clang socat meson libdbus-1-dev e2fslibs-dev
+        # One of the tests wants this
+        sudo mkdir /tmp/flatpak-com.example.App-OwnedByRoot
     - name: Check out flatpak
       uses: actions/checkout@v1
       with:

common/flatpak-context-private.h

@@ -57,6 +57,7 @@ typedef enum {
   FLATPAK_CONTEXT_FEATURE_MULTIARCH    = 1 << 1,
   FLATPAK_CONTEXT_FEATURE_BLUETOOTH    = 1 << 2,
   FLATPAK_CONTEXT_FEATURE_CANBUS       = 1 << 3,
+  FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM = 1 << 4,
 } FlatpakContextFeatures;
 
 struct FlatpakContext

common/flatpak-context.c

@@ -78,6 +78,7 @@ const char *flatpak_context_features[] = {
   "multiarch",
   "bluetooth",
   "canbus",
+  "per-app-dev-shm",
   NULL
 };
 
@@ -2064,6 +2065,11 @@ gboolean
 flatpak_context_adds_permissions (FlatpakContext *old,
                                   FlatpakContext *new)
 {
+  /* We allow upgrade to multiarch, that is really not a huge problem.
+   * Similarly, having sensible semantics for /dev/shm is
+   * not a security concern. */
+  guint32 harmless_features = (FLATPAK_CONTEXT_FEATURE_MULTIARCH |
+                               FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM);
   guint32 old_sockets;
 
   if (adds_flags (old->shares & old->shares_valid,
@@ -2085,8 +2091,7 @@ flatpak_context_adds_permissions (FlatpakContext *old,
                   new->devices & new->devices_valid))
     return TRUE;
 
-  /* We allow upgrade to multiarch, that is really not a huge problem */
-  if (adds_flags ((old->features & old->features_valid) | FLATPAK_CONTEXT_FEATURE_MULTIARCH,
+  if (adds_flags ((old->features & old->features_valid) | harmless_features,
                   new->features & new->features_valid))
     return TRUE;
 

common/flatpak-instance-private.h

@@ -30,12 +30,24 @@ char *flatpak_instance_get_instances_directory (void);
 char *flatpak_instance_allocate_id (char **host_dir_out,
                                     int *lock_fd_out);
 
+gboolean flatpak_instance_claim_per_app_temp_directory (const char *app_id,
+                                                        int per_app_dir_lock_fd,
+                                                        int at_fd,
+                                                        const char *link_path,
+                                                        const char *parent,
+                                                        char **path_out,
+                                                        GError **error);
+
 void flatpak_instance_iterate_all_and_gc (GPtrArray *out_instances);
 
 gboolean flatpak_instance_ensure_per_app_dir (const char *app_id,
                                               int *lock_fd_out,
                                               char **lock_path_out,
                                               GError **error);
+gboolean flatpak_instance_ensure_per_app_dev_shm (const char *app_id,
+                                                  int per_app_dir_lock_fd,
+                                                  char **shared_dev_shm_out,
+                                                  GError **error);
 gboolean flatpak_instance_ensure_per_app_tmp (const char *app_id,
                                               int per_app_dir_lock_fd,
                                               char **shared_tmp_out,

common/flatpak-instance.c

@@ -20,6 +20,10 @@
 
 #include "config.h"
 
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "flatpak-utils-base-private.h"
 #include "flatpak-utils-private.h"
 #include "flatpak-run-private.h"
 #include "flatpak-instance.h"
@@ -530,6 +534,102 @@ flatpak_instance_ensure_per_app_dir (const char *app_id,
   return TRUE;
 }
 
+/*
+ * @app_id: $FLATPAK_ID
+ * @per_app_dir_lock_fd: Used to prove that we have already taken out
+ *  a per-app non-exclusive lock to stop this directory from being
+ *  garbage-collected
+ * @shared_tmp: (out) (not optional): Used to return the path to the
+ *  shared /dev/shm
+ *
+ * Create the per-app /dev/shm.
+ */
+gboolean
+flatpak_instance_ensure_per_app_dev_shm (const char *app_id,
+                                         int per_app_dir_lock_fd,
+                                         char **shared_dev_shm_out,
+                                         GError **error)
+{
+  /* This function is actually generic, since we might well want to
+   * offload other directories in the same way - but the only directory
+   * we do this for right now is /dev/shm. */
+  static const char link_name[] = "dev-shm";
+  static const char parent[] = "/dev/shm";
+  g_autofree gchar *flag_file = NULL;
+  g_autofree gchar *path = NULL;
+  g_autofree gchar *per_app_parent = NULL;
+  g_autofree gchar *per_app_dir = NULL;
+  glnx_autofd int flag_fd = -1;
+  glnx_autofd int per_app_dir_fd = -1;
+
+  g_return_val_if_fail (app_id != NULL, FALSE);
+  g_return_val_if_fail (per_app_dir_lock_fd >= 0, FALSE);
+  g_return_val_if_fail (shared_dev_shm_out != NULL, FALSE);
+  g_return_val_if_fail (*shared_dev_shm_out == NULL, FALSE);
+  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+  per_app_parent = flatpak_instance_get_apps_directory ();
+
+  per_app_dir = g_build_filename (per_app_parent, app_id, NULL);
+  per_app_dir_fd = openat (AT_FDCWD, per_app_dir,
+                           O_PATH | O_DIRECTORY | O_CLOEXEC);
+
+  /* This can't happen under normal circumstances: if we have the lock,
+   * then the directory it's in had better exist. */
+  if (per_app_dir_fd < 0)
+    return glnx_throw_errno_prefix (error,
+                                    _("Unable to open directory %s"),
+                                    per_app_dir);
+
+  /* If there's an existing symlink to a suitable directory, we can
+   * reuse it (carefully). This gives us the sharing we wanted between
+   * multiple instances of the same app, and between app and subsandbox. */
+  if (flatpak_instance_claim_per_app_temp_directory (app_id,
+                                                     per_app_dir_lock_fd,
+                                                     per_app_dir_fd,
+                                                     link_name,
+                                                     parent,
+                                                     &path,
+                                                     NULL))
+    {
+      *shared_dev_shm_out = g_steal_pointer (&path);
+      return TRUE;
+    }
+
+  /* Otherwise create a new directory in @parent, and make @link_name
+   * a symlink to it. */
+
+  /* /dev/shm/flatpak-$FLATPAK_ID-XXXXXX */
+  path = g_strdup_printf ("%s/flatpak-%s-XXXXXX", parent, app_id);
+
+  if (g_mkdtemp (path) == NULL)
+    return glnx_throw_errno_prefix (error,
+                                    _("Unable to create temporary directory in %s"),
+                                    parent);
+
+  /* This marks this directory as an expendable temp directory, and is
+   * inspired by the use of .testtmp in libostree. */
+  flag_file = g_build_filename (path, ".flatpak-tmpdir", NULL);
+  flag_fd = openat (AT_FDCWD, flag_file,
+                    O_RDONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW | O_NOCTTY,
+                    0600);
+
+  if (flag_fd < 0)
+    return glnx_throw_errno_prefix (error,
+                                    _("Unable to create file %s"),
+                                    path);
+
+  /* Replace the symlink */
+  if ((unlinkat (per_app_dir_fd, link_name, 0) < 0 && errno != ENOENT) ||
+      symlinkat (path, per_app_dir_fd, link_name) < 0)
+    return glnx_throw_errno_prefix (error,
+                                    _("Unable to update symbolic link %s/%s"),
+                                    per_app_dir, link_name);
+
+  *shared_dev_shm_out = g_steal_pointer (&path);
+  return TRUE;
+}
+
 /*
  * @app_id: $FLATPAK_ID
  * @per_app_dir_lock_fd: Used to prove that we have already taken out
@@ -645,6 +745,130 @@ flatpak_instance_new_for_id (const char *id)
   return flatpak_instance_new (dir);
 }
 
+/*
+ * flatpak_instance_claim_per_app_temp_directory:
+ * @app_id: $FLATPAK_ID
+ * @per_app_dir_lock_fd: Lock representing shared or exclusive access
+ *  to directories created for @app_id
+ * @at_fd: Directory in which to look up @link_path
+ * @link_path: Path of a symbolic link to a subdirectory of @parent
+ * @parent: The directory in which we created the temporary directory,
+ *  such as /dev/shm or /tmp
+ * @path_out: (out) (not optional): Return the path to the directory
+ *  referenced by @link_path
+ *
+ * Try to take control of an existing per-app temporary directory
+ * referenced by @link_path, either for reuse or for deletion.
+ * Return %TRUE if we can.
+ *
+ * This is currently only used for /dev/shm, but it's designed to be
+ * equally usable for other non-user-owned directories like /tmp.
+ *
+ * We have to be careful here, because @link_path might be left over
+ * from a previous boot, and it probably points into a directory like
+ * /dev/shm or /tmp, where an attacker might recreate our directories,
+ * for example as symbolic links to somewhere they control. As a result,
+ * this function is security-sensitive, and needs to follow a policy of
+ * failing when an unexpected situation is detected.
+ *
+ * @error is not normally user-visible, and is mostly present to support
+ * debugging and unit testing.
+ *
+ * Returns: %TRUE if @link_path points to a suitable directory,
+ *  or %FALSE with @error set if it does not.
+ */
+gboolean
+flatpak_instance_claim_per_app_temp_directory (const char *app_id,
+                                               int per_app_dir_lock_fd,
+                                               int at_fd,
+                                               const char *link_path,
+                                               const char *parent,
+                                               char **path_out,
+                                               GError **error)
+{
+  g_autofree char *reuse_path = NULL;
+  glnx_autofd int dfd = -1;
+  glnx_autofd int flag_fd = -1;
+  struct stat statbuf;
+  const char *slash;
+  const char *rest;
+
+  at_fd = glnx_dirfd_canonicalize (at_fd);
+
+  g_return_val_if_fail (app_id != NULL, FALSE);
+  g_return_val_if_fail (per_app_dir_lock_fd >= 0, FALSE);
+  g_return_val_if_fail (at_fd == AT_FDCWD || at_fd >= 0, FALSE);
+  g_return_val_if_fail (link_path != NULL, FALSE);
+  g_return_val_if_fail (parent != NULL, FALSE);
+  g_return_val_if_fail (path_out != NULL, FALSE);
+  g_return_val_if_fail (*path_out == NULL, FALSE);
+  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+  reuse_path = glnx_readlinkat_malloc (at_fd, link_path, NULL, error);
+
+  if (reuse_path == NULL)
+    return FALSE;
+
+  /* If we're going to use it as /dev/shm, the directory on the
+   * host should match /dev/shm/flatpak-$FLATPAK_ID-XXXXXX */
+  if (!g_str_has_prefix (reuse_path, parent))
+    return glnx_throw (error, "%s does not start with %s",
+                       reuse_path, parent);
+
+  /* /flatpak-$FLATPAK_ID-XXXXXX */
+  slash = reuse_path + strlen (parent);
+
+  if (*slash != '/')
+    return glnx_throw (error, "%s does not start with %s/",
+                       reuse_path, parent);
+
+  /* flatpak-$FLATPAK_ID-XXXXXX */
+  rest = slash + 1;
+
+  if (!g_str_has_prefix (rest, "flatpak-"))
+    return glnx_throw (error, "%s does not start with %s/flatpak-",
+                       reuse_path, parent);
+
+  if (strchr (rest, '/') != NULL)
+    return glnx_throw (error, "%s has too many directory separators",
+                       reuse_path);
+
+  if (!g_str_has_prefix (rest + strlen ("flatpak-"), app_id))
+    return glnx_throw (error, "%s does not start with %s/flatpak-%s",
+                       reuse_path, parent, app_id);
+
+  if (rest[strlen ("flatpak-") + strlen (app_id)] != '-')
+    return glnx_throw (error, "%s does not start with %s/flatpak-%s-",
+                       reuse_path, parent, app_id);
+
+  /* Avoid symlink attacks via O_NOFOLLOW */
+  dfd = openat (AT_FDCWD, reuse_path,
+                O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
+
+  if (dfd < 0)
+    return glnx_throw_errno_prefix (error, "opening %s O_DIRECTORY|O_NOFOLLOW",
+                                    reuse_path);
+
+  if (fstat (dfd, &statbuf) < 0)
+    return glnx_throw_errno_prefix (error, "fstat %s", reuse_path);
+
+  /* We certainly don't want to reuse someone else's directory */
+  if (statbuf.st_uid != geteuid ())
+    return glnx_throw (error, "%s does not belong to this user", reuse_path);
+
+  flag_fd = openat (dfd, ".flatpak-tmpdir",
+                    O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NOCTTY);
+
+  /* If we can't open the flag file, the most likely reason is that it
+   * isn't a directory that we created */
+  if (flag_fd < 0)
+    return glnx_throw_errno_prefix (error, "opening flag file %s/.flatpak-tmpdir",
+                                    reuse_path);
+
+  *path_out = g_steal_pointer (&reuse_path);
+  return TRUE;
+}
+
 /*
  * The @error is not intended to be user-facing, and is there for
  * testing/debugging.
@@ -726,6 +950,61 @@ flatpak_instance_gc_per_app_dirs (const char *instance_id,
 
   g_debug ("Cleaning up per-app-ID state for %s", app_id);
 
+  /* /dev/shm is offloaded onto the host's /dev/shm to get consistent
+   * free space behaviour and make sure it's actually in RAM. It could
+   * contain relatively large files, so we clean it up.
+   *
+   * In principle this could be used for other directories such as /tmp,
+   * in a loop over an array of paths (hence this indentation), but we
+   * only do this for /dev/shm right now. */
+  do
+    {
+      g_autofree char *path = NULL;
+
+      /* /dev/shm is an attacker-controlled namespace, so we need to be
+       * careful what directories we will delete. We have to assume
+       * that attackers will create malicious symlinks in /dev/shm to
+       * try to trick us into opening or deleting the wrong files. */
+      if (flatpak_instance_claim_per_app_temp_directory (app_id,
+                                                         per_app_dir_lock_fd,
+                                                         per_app_dir_fd,
+                                                         "dev-shm",
+                                                         "/dev/shm",
+                                                         &path,
+                                                         &local_error))
+        {
+          g_assert (g_str_has_prefix (path, "/dev/shm/"));
+
+          if (unlinkat (per_app_dir_fd, "dev-shm", 0) != 0)
+            g_debug ("Unable to clean up %s/%s: %s",
+                     per_app_dir, "dev-shm", g_strerror (errno));
+
+          if (!glnx_shutil_rm_rf_at (AT_FDCWD, path, NULL, &local_error))
+            {
+              g_debug ("Unable to clean up %s: %s",
+                       path, local_error->message);
+              g_clear_error (&local_error);
+            }
+        }
+      else if (unlinkat (per_app_dir_fd, "dev-shm", 0) < 0 && errno == ENOENT)
+        {
+          /* ignore, the symlink wasn't even there anyway */
+          g_clear_error (&local_error);
+        }
+      else
+        {
+          g_debug ("%s/%s no longer points to the expected directory and "
+                   "was removed: %s",
+                   per_app_dir, "dev-shm", local_error->message);
+          g_clear_error (&local_error);
+        }
+    }
+  while (0);
+
+  /* We currently allocate the app's /tmp directly in the per-app directory
+   * on the host's XDG_RUNTIME_DIR, instead of offloading it into /tmp
+   * in a way that's analogous to /dev/shm, so we expect tmp to be a directory
+   * and not a symlink. If it's a symlink, we'll just unlink it. */
   if (!glnx_shutil_rm_rf_at (per_app_dir_fd, "tmp", NULL, &local_error))
     {
       g_debug ("Unable to clean up %s/tmp: %s", per_app_dir,
@@ -780,9 +1059,14 @@ flatpak_instance_iterate_all_and_gc (GPtrArray *out_instances)
               fcntl (lock_fd, F_GETLK, &l) == 0 &&
               l.l_type == F_UNLCK)
             {
+              g_autoptr(GError) local_error = NULL;
+
               /* The instance is not used, remove it */
               g_debug ("Cleaning up unused container id %s", dent->d_name);
-              flatpak_instance_gc_per_app_dirs (dent->d_name, NULL);
+
+              if (!flatpak_instance_gc_per_app_dirs (dent->d_name, &local_error))
+                flatpak_debug2 ("Not cleaning up per-app dir: %s", local_error->message);
+
               glnx_shutil_rm_rf_at (iter.fd, dent->d_name, NULL, NULL);
               continue;
             }

common/flatpak-run.c

@@ -1401,10 +1401,39 @@ flatpak_run_add_environment_args (FlatpakBwrap    *bwrap,
       /* Don't expose the host /dev/shm, just the device nodes, unless explicitly allowed */
       if (g_file_test ("/dev/shm", G_FILE_TEST_IS_DIR))
         {
-          if ((context->devices & FLATPAK_CONTEXT_DEVICE_SHM) == 0)
-            flatpak_bwrap_add_args (bwrap,
-                                    "--tmpfs", "/dev/shm",
-                                    NULL);
+          if (context->devices & FLATPAK_CONTEXT_DEVICE_SHM)
+            {
+              /* Don't do anything special: include shm in the
+               * shared /dev. The host and all sandboxes and subsandboxes
+               * all share /dev/shm */
+            }
+          else if ((context->features & FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM)
+                   && per_app_dir_lock_fd >= 0)
+            {
+              g_autofree char *shared_dev_shm = NULL;
+
+              /* The host and the original sandbox have separate /dev/shm,
+               * but we want other instances to be able to share /dev/shm with
+               * the first sandbox (except for subsandboxes run with
+               * flatpak-spawn --sandbox, which will have their own). */
+              if (!flatpak_instance_ensure_per_app_dev_shm (app_id,
+                                                            per_app_dir_lock_fd,
+                                                            &shared_dev_shm,
+                                                            error))
+                return FALSE;
+
+              flatpak_bwrap_add_args (bwrap,
+                                      "--bind", shared_dev_shm, "/dev/shm",
+                                      NULL);
+            }
+          else
+            {
+              /* The host, the original sandbox and each subsandbox
+               * each have a separate /dev/shm. */
+              flatpak_bwrap_add_args (bwrap,
+                                      "--tmpfs", "/dev/shm",
+                                      NULL);
+            }
         }
       else if (g_file_test ("/dev/shm", G_FILE_TEST_IS_SYMLINK))
         {
@@ -1416,13 +1445,35 @@ flatpak_run_add_environment_args (FlatpakBwrap    *bwrap,
             {
               if (context->devices & FLATPAK_CONTEXT_DEVICE_SHM &&
                   g_file_test ("/run/shm", G_FILE_TEST_IS_DIR))
-                flatpak_bwrap_add_args (bwrap,
-                                        "--bind", "/run/shm", "/run/shm",
-                                        NULL);
+                {
+                  flatpak_bwrap_add_args (bwrap,
+                                          "--bind", "/run/shm", "/run/shm",
+                                          NULL);
+                }
+              else if ((context->features & FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM)
+                       && per_app_dir_lock_fd >= 0)
+                {
+                  g_autofree char *shared_dev_shm = NULL;
+
+                  /* The host and the original sandbox have separate /dev/shm,
+                   * but we want other instances to be able to share /dev/shm,
+                   * except for flatpak-spawn --subsandbox. */
+                  if (!flatpak_instance_ensure_per_app_dev_shm (app_id,
+                                                                per_app_dir_lock_fd,
+                                                                &shared_dev_shm,
+                                                                error))
+                    return FALSE;
+
+                  flatpak_bwrap_add_args (bwrap,
+                                          "--bind", shared_dev_shm, "/run/shm",
+                                          NULL);
+                }
               else
-                flatpak_bwrap_add_args (bwrap,
-                                        "--dir", "/run/shm",
-                                        NULL);
+                {
+                  flatpak_bwrap_add_args (bwrap,
+                                          "--dir", "/run/shm",
+                                          NULL);
+                }
             }
           else
             g_warning ("Unexpected /dev/shm symlink %s", link);
@@ -1484,8 +1535,22 @@ flatpak_run_add_environment_args (FlatpakBwrap    *bwrap,
           if (real_dev_shm != NULL)
               flatpak_bwrap_add_args (bwrap, "--bind", real_dev_shm, "/dev/shm", NULL);
         }
-    }
+      else if ((context->features & FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM)
+               && per_app_dir_lock_fd >= 0)
+        {
+          g_autofree char *shared_dev_shm = NULL;
 
+          if (!flatpak_instance_ensure_per_app_dev_shm (app_id,
+                                                        per_app_dir_lock_fd,
+                                                        &shared_dev_shm,
+                                                        error))
+            return FALSE;
+
+          flatpak_bwrap_add_args (bwrap,
+                                  "--bind", shared_dev_shm, "/dev/shm",
+                                  NULL);
+        }
+    }
 
   exports = flatpak_context_get_exports_full (context,
                                               app_id_dir, previous_app_id_dirs,

doc/flatpak-build-finish.xml

@@ -184,7 +184,8 @@
                 <listitem><para>
                     Allow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    FEATURE must be one of: devel, multiarch, bluetooth, canbus.
+                    FEATURE must be one of: devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                  </para><para>
                     The <code>devel</code> feature allows the application to
@@ -205,6 +206,13 @@
                     The <code>canbus</code> feature allows the application to
                     use canbus (AF_CAN) sockets.
                     Note, for this work you must also have network access.
+                </para>
+                <para>
+                    The <code>per-app-dev-shm</code> feature shares a single
+                    instance of <filename>/dev/shm</filename> between the
+                    application, any unrestricted subsandboxes that it creates,
+                    and any other instances of the application that are
+                    launched while it is running.
                 </para></listitem>
             </varlistentry>
 
@@ -214,7 +222,8 @@
                 <listitem><para>
                     Disallow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    FEATURE must be one of: devel, multiarch, bluetooth, canbus.
+                    FEATURE must be one of: devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                 </para></listitem>
             </varlistentry>

doc/flatpak-build.xml

@@ -194,7 +194,9 @@
                 <listitem><para>
                     Allow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    <arg choice="plain">FEATURE</arg> must be one of: devel, multiarch, bluetooth, canbus.
+                    <arg choice="plain">FEATURE</arg> must be one of:
+                    devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                     </para><para>
                     See <citerefentry><refentrytitle>flatpak-build-finish</refentrytitle><manvolnum>1</manvolnum></citerefentry>
@@ -208,7 +210,9 @@
                 <listitem><para>
                     Disallow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    <arg choice="plain">FEATURE</arg> must be one of: devel, multiarch, bluetooth, canbus.
+                    <arg choice="plain">FEATURE</arg> must be one of:
+                    devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                 </para></listitem>
             </varlistentry>

doc/flatpak-metadata.xml

@@ -478,6 +478,19 @@
                               Available since 1.0.3.
                             </para></listitem></varlistentry>
 
+                            <varlistentry><term><option>per-app-dev-shm</option></term>
+                            <listitem><para>
+                              Share a single instance of
+                              <filename>/dev/shm</filename> between all
+                              instances of this application run by the same
+                              user ID, including sub-sandboxes.
+                              If the application has the
+                              <option>shm</option> device permission in its
+                              <option>devices</option> list, then this
+                              feature flag is ignored.
+                              Available since 1.12.0.
+                            </para></listitem></varlistentry>
+
                         </variablelist>
                         A feature can be prefixed with <option>!</option> to
                         indicate the absence of that feature, for example

doc/flatpak-override.xml

@@ -176,7 +176,9 @@
                 <listitem><para>
                     Allow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    <arg choice="plain">FEATURE</arg> must be one of: devel, multiarch, bluetooth, canbus.
+                    <arg choice="plain">FEATURE</arg> must be one of:
+                    devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                  </para><para>
                     See <citerefentry><refentrytitle>flatpak-build-finish</refentrytitle><manvolnum>1</manvolnum></citerefentry>
@@ -190,7 +192,9 @@
                 <listitem><para>
                     Disallow access to a specific feature. This updates
                     the [Context] group in the metadata.
-                    <arg choice="plain">FEATURE</arg> must be one of: devel, multiarch, bluetooth, canbus.
+                    <arg choice="plain">FEATURE</arg> must be one of:
+                    devel, multiarch, bluetooth, canbus,
+                    per-app-dev-shm.
                     This option can be used multiple times.
                 </para></listitem>
             </varlistentry>

tests/Makefile.am.inc

@@ -88,7 +88,11 @@ test_exports_SOURCES = tests/test-exports.c
 
 test_instance_CFLAGS = $(testcommon_CFLAGS)
 test_instance_LDADD = $(testcommon_LDADD) libtestlib.la
-test_instance_SOURCES = tests/test-instance.c
+test_instance_SOURCES = \
+	subprojects/libglnx/tests/libglnx-testlib.c \
+	subprojects/libglnx/tests/libglnx-testlib.h \
+	tests/test-instance.c \
+	$(NULL)
 
 tests_hold_lock_CFLAGS = $(AM_CFLAGS) $(BASE_CFLAGS)
 tests_hold_lock_LDADD = $(AM_LDADD) $(BASE_LIBS) libglnx.la

tests/test-completion.sh

@@ -122,6 +122,7 @@ bluetooth
 canbus 
 devel 
 multiarch 
+per-app-dev-shm 
 EOF
 
 ok "complete --allow"

tests/test-exports.c

@@ -233,7 +233,7 @@ test_full_context (void)
   g_key_file_set_value (keyfile,
                         FLATPAK_METADATA_GROUP_CONTEXT,
                         FLATPAK_METADATA_KEY_FEATURES,
-                        "devel;multiarch;bluetooth;canbus;");
+                        "devel;multiarch;bluetooth;canbus;per-app-dev-shm;");
   g_key_file_set_value (keyfile,
                         FLATPAK_METADATA_GROUP_CONTEXT,
                         FLATPAK_METADATA_KEY_FILESYSTEMS,
@@ -292,7 +292,8 @@ test_full_context (void)
                     (FLATPAK_CONTEXT_FEATURE_DEVEL |
                      FLATPAK_CONTEXT_FEATURE_MULTIARCH |
                      FLATPAK_CONTEXT_FEATURE_BLUETOOTH |
-                     FLATPAK_CONTEXT_FEATURE_CANBUS));
+                     FLATPAK_CONTEXT_FEATURE_CANBUS |
+                     FLATPAK_CONTEXT_FEATURE_PER_APP_DEV_SHM));
   g_assert_cmpuint (context->features_valid, ==, context->features);
 
   g_assert_cmpuint (flatpak_context_get_run_flags (context), ==,

tests/test-instance.c

@@ -33,6 +33,7 @@
 #include "flatpak-run-private.h"
 
 #include "libglnx/libglnx.h"
+#include "libglnx/tests/libglnx-testlib.h"
 
 #include "testlib.h"
 
@@ -215,6 +216,274 @@ test_gc (void)
   g_spawn_close_pid (pid);
 }
 
+static void
+test_claim_per_app_temp_directory (void)
+{
+  /* Run in a temporary directory so we can create a bunch of symlinks */
+  _GLNX_TEST_SCOPED_TEMP_DIR;
+
+  gboolean ok;
+  glnx_autofd int lock_fd = -1;
+  glnx_autofd int fd = -1;
+  g_autofree char *result = NULL;
+  g_autofree char *flag_path = NULL;
+  g_autofree char *symlink_path = NULL;
+  g_autofree char *non_directory_path = NULL;
+  g_autofree char *dir_in_tmp = NULL;
+  g_autoptr(GError) error = NULL;
+  struct stat stat_buf;
+
+  /* In real life this would be the per-app-ID lock, but in fact
+   * we just need some sort of file descriptor - as currently
+   * implemented, we don't even need to lock it. */
+  lock_fd = open ("mock-per-app-id-lock",
+                  O_CLOEXEC | O_CREAT | O_NOCTTY | O_NOFOLLOW,
+                  0600);
+  g_assert_no_errno (lock_fd >= 0 ? 0 : -1);
+
+  /* This emulates the sort of directory that we want to reuse. */
+  dir_in_tmp = g_strdup ("/tmp/flatpak-com.example.App-XXXXXX");
+  g_assert_nonnull (g_mkdtemp (dir_in_tmp));
+
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "doesnt-exist",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND);
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* If link_path is a symlink to a directory not in /tmp, we refuse
+   * to reuse it */
+  g_assert_no_errno (symlink ("/nope", "bad-prefix"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "bad-prefix",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==, "/nope does not start with /tmp");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* Similar */
+  g_assert_no_errno (symlink ("/tmptation", "bad-prefix2"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "bad-prefix2",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==, "/tmptation does not start with /tmp/");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* If link_path points to a subdirectory of /tmp that doesn't match the
+   * expected pattern, we refuse to reuse it */
+  g_assert_no_errno (symlink ("/tmp/nope", "bad-prefix3"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "bad-prefix3",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==, "/tmp/nope does not start with /tmp/flatpak-");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* Similar */
+  g_assert_no_errno (symlink ("/tmp/flatpak-/nope", "too-many-levels"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "too-many-levels",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==,
+                   "/tmp/flatpak-/nope has too many directory separators");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* Similar */
+  g_assert_no_errno (symlink ("/tmp/flatpak-abc/", "too-many-levels2"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "too-many-levels2",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==,
+                   "/tmp/flatpak-abc/ has too many directory separators");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  g_assert_no_errno (symlink ("/tmp/flatpak-org.example.Other-XXXXXX", "wrong-app"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "wrong-app",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==,
+                   "/tmp/flatpak-org.example.Other-XXXXXX does not "
+                   "start with /tmp/flatpak-com.example.App");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  g_assert_no_errno (symlink ("/tmp/flatpak-com.example.ApparentlyNot", "wrong-app2"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "wrong-app2",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_nonnull (error);
+  g_assert_cmpstr (error->message, ==,
+                   "/tmp/flatpak-com.example.ApparentlyNot does not "
+                   "start with /tmp/flatpak-com.example.App-");
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* If it points to a filesystem object matching the right pattern, but
+   * that is not a directory, we refuse to reuse it */
+  non_directory_path = g_strdup ("/tmp/flatpak-com.example.App-XXXXXX");
+  g_assert_no_errno ((fd = g_mkstemp (non_directory_path)));
+  g_assert_no_errno (symlink (non_directory_path, "not-a-directory"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "not-a-directory",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY);
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* Reuse @non_directory_path as the name of a symlink to a directory:
+   * we consider that to be equally invalid. Create it inside our
+   * directory in /tmp so that we can rename() it into place,
+   * because symlink() does not overwrite, but rename() does. */
+  symlink_path = g_build_filename (dir_in_tmp, "symlink", NULL);
+  g_assert_no_errno (symlink (dir_in_tmp, symlink_path));
+  /* Overwrite the file with the symlink */
+  g_assert_no_errno (rename (symlink_path, non_directory_path));
+
+  /* We'll refuse to follow the symlink: for all we know it could be
+   * attacker-controlled. */
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "not-a-directory",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+
+  /* Either of these would be reasonable */
+  if (error->code == G_IO_ERROR_TOO_MANY_LINKS)
+    g_assert_error (error, G_IO_ERROR, G_IO_ERROR_TOO_MANY_LINKS);
+  else
+    g_assert_error (error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY);
+
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* If link_path points to a directory owned by someone else, we refuse
+   * to use it. This part of the test will be skipped unless you pre-create
+   * this directory as root. */
+  if (stat ("/tmp/flatpak-com.example.App-OwnedByRoot", &stat_buf) == 0
+      && stat_buf.st_uid == 0
+      && geteuid () != 0)
+    {
+      g_assert_no_errno (symlink ("/tmp/flatpak-com.example.App-OwnedByRoot",
+                                  "not-our-directory"));
+      ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                          lock_fd,
+                                                          AT_FDCWD,
+                                                          "not-our-directory",
+                                                          "/tmp",
+                                                          &result,
+                                                          &error);
+      g_assert_nonnull (error);
+      g_assert_cmpstr (error->message, ==,
+                       "/tmp/flatpak-com.example.App-OwnedByRoot does not "
+                       "belong to this user");
+      g_assert_null (result);
+      g_assert_false (ok);
+      g_clear_error (&error);
+    }
+
+  glnx_close_fd (&fd);
+  g_assert_no_errno (unlink (non_directory_path));
+  g_clear_pointer (&non_directory_path, g_free);
+
+  /* Even when we have a symlink to a directory matching the right pattern
+   * that we own, if it doesn't contain the flag file that indicates that
+   * it's one of our temp directories, we'll still refuse to use it. */
+  g_assert_no_errno (symlink (dir_in_tmp, "good-symlink"));
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "good-symlink",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND);
+  g_assert_true (g_str_has_prefix (error->message,
+                                   "opening flag file /tmp/flatpak-com.example.App-"));
+  g_assert_nonnull (strstr (error->message, "/.flatpak-tmpdir:"));
+  g_assert_null (result);
+  g_assert_false (ok);
+  g_clear_error (&error);
+
+  /* Create the flag file (of course in real life this would have happened
+   * much sooner) */
+  flag_path = g_build_filename (dir_in_tmp, ".flatpak-tmpdir", NULL);
+  g_file_set_contents (flag_path, "", 0, &error);
+  g_assert_no_error (error);
+
+  /* Now we are finally willing to reuse the directory! A happy ending
+   * at last. */
+  ok = flatpak_instance_claim_per_app_temp_directory ("com.example.App",
+                                                      lock_fd,
+                                                      AT_FDCWD,
+                                                      "good-symlink",
+                                                      "/tmp",
+                                                      &result,
+                                                      &error);
+  g_assert_no_error (error);
+  g_assert_cmpstr (result, ==, dir_in_tmp);
+  g_assert_true (ok);
+
+  g_assert_no_errno (unlink (flag_path));
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -225,6 +494,8 @@ main (int argc, char *argv[])
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/instance/gc", test_gc);
+  g_test_add_func ("/instance/claim-per-app-temp-directory",
+                   test_claim_per_app_temp_directory);
 
   res = g_test_run ();