From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Mon, 21 Aug 2023 17:32:21 +0000 (+0200)
Subject: mount-tool: override mode of --tmpfs mounts to (rwxrwxrwx & ~umask)
X-Git-Tag: v255-rc1~646^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9269296df05a724c6b121bcbb0b420c8b4088f2f;p=thirdparty%2Fsystemd.git

mount-tool: override mode of --tmpfs mounts to (rwxrwxrwx & ~umask)

The kernel default for tmpfs mounts is rwxrwxrwt, i.e. world-writable with the
sticky bit set. This makes sense for /tmp and /var/tmp, but less so for other
directories. Users will not use systemd-mount to mount /tmp and /var/tmp, so
the existing behaviour is surprising. The defaults are overriden to the
same defaults we use for "internal tmpfs mounts", i.e. no sticky bit, nosuid,
nodev, and limits of size=10%, nr_inodes=400k.

--tmpfs is new, so we can modify the behaviour without breaking compatibility.
---

diff --git a/man/systemd-mount.xml b/man/systemd-mount.xml
index 3463a107dab..dfa4a95cb1c 100644
--- a/man/systemd-mount.xml
+++ b/man/systemd-mount.xml
@@ -287,8 +287,17 @@
         <term><option>--tmpfs</option></term>
 
         <listitem>
-          <para>Create and mount a new temporary file system on <replaceable>WHERE</replaceable>, with an
-          optional <replaceable>NAME</replaceable> that defaults to <literal>tmpfs</literal>.</para>
+          <para>Create and mount a new <constant>tmpfs</constant> file system on
+          <replaceable>WHERE</replaceable>, with an optional <replaceable>NAME</replaceable> that defaults to
+          <literal>tmpfs</literal>.</para>
+
+          <para>The file system is mounted with the top-level directory mode determined by the
+          <citerefentry><refentrytitle>umask</refentrytitle><manvolnum>2</manvolnum></citerefentry> setting
+          of the caller, i.e. <constant>rwxrwxrwx</constant> masked by the umask of the caller. This matches
+          what
+          <citerefentry project='man-pages'><refentrytitle>mkdir</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+          does, but is different from the kernel default of <literal>rwxrwxrwxt</literal>, i.e. a
+          world-writable directory with the sticky bit set.</para>
         </listitem>
       </varlistentry>
 
diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c
index 780179b3ed2..b167176a9f5 100644
--- a/src/mount/mount-tool.c
+++ b/src/mount/mount-tool.c
@@ -35,6 +35,7 @@
 #include "stat-util.h"
 #include "strv.h"
 #include "terminal-util.h"
+#include "umask-util.h"
 #include "unit-def.h"
 #include "unit-name.h"
 #include "user-util.h"
@@ -506,7 +507,6 @@ static int transient_unit_set_properties(sd_bus_message *m, UnitType t, char **p
 }
 
 static int transient_mount_set_properties(sd_bus_message *m) {
-        _cleanup_free_ char *options = NULL;
         int r;
 
         assert(m);
@@ -527,20 +527,43 @@ static int transient_mount_set_properties(sd_bus_message *m) {
                         return r;
         }
 
+        _cleanup_free_ char *options = NULL;
+
         /* Prepend uid=…,gid=… if arg_uid is set */
         if (arg_uid != UID_INVALID) {
-                r = asprintf(&options,
-                             "uid=" UID_FMT ",gid=" GID_FMT "%s%s",
-                             arg_uid, arg_gid,
-                             arg_mount_options ? "," : "", strempty(arg_mount_options));
+                r = strextendf_with_separator(&options, ",",
+                                              "uid="UID_FMT",gid="GID_FMT, arg_uid, arg_gid);
+                if (r < 0)
+                        return r;
+        }
+
+        /* Override the default for tmpfs mounts. The kernel sets the sticky bit on the root directory by
+         * default. This makes sense for the case when the user does 'mount -t tmpfs tmpfs /tmp', but less so
+         * for other directories.
+         *
+         * Let's also set some reasonable limits. We use the current umask, to match what a command to create
+         * directory would use, e.g. mkdir. */
+        if (arg_tmpfs) {
+                mode_t mask;
+
+                r = get_process_umask(0, &mask);
+                if (r < 0)
+                        return r;
+
+                assert((mask & ~0777) == 0);
+                r = strextendf_with_separator(&options, ",",
+                                              "mode=0%o,nodev,nosuid%s", 0777 & ~mask, NESTED_TMPFS_LIMITS);
                 if (r < 0)
-                        return -ENOMEM;
+                        return r;
         }
 
-        if (options || arg_mount_options) {
-                log_debug("Using mount options: %s", options ?: arg_mount_options);
+        if (arg_mount_options)
+                if (!strextend_with_separator(&options, ",", arg_mount_options))
+                        return r;
 
-                r = sd_bus_message_append(m, "(sv)", "Options", "s", options ?: arg_mount_options);
+        if (options) {
+                log_debug("Using mount options: %s", options);
+                r = sd_bus_message_append(m, "(sv)", "Options", "s", options);
                 if (r < 0)
                         return r;
         } else