core/namespace: replace MOUNT_PRIVATE_TMP_READ_ONLY with MOUNT_PRIVATE_TMP with .read_only = true

This also adds test for MountAPIVFS=yes.

For issue #34825.

src/core/namespace.c

@@ -59,7 +59,6 @@ typedef enum MountMode {
         MOUNT_BIND,
         MOUNT_BIND_RECURSIVE,
         MOUNT_PRIVATE_TMP,
-        MOUNT_PRIVATE_TMP_READ_ONLY,
         MOUNT_PRIVATE_DEV,
         MOUNT_BIND_DEV,
         MOUNT_EMPTY_DIR,
@@ -221,7 +220,7 @@ static const MountEntry protect_system_full_table[] = {
  * left writable, as ProtectHome= shall manage those, orthogonally).
  */
 static const MountEntry protect_system_strict_table[] = {
-        { "/",                   MOUNT_READ_ONLY,          false },
+        { "/",                   MOUNT_READ_ONLY,           false },
         { "/proc",               MOUNT_READ_WRITE_IMPLICIT, false },      /* ProtectKernelTunables= */
         { "/sys",                MOUNT_READ_WRITE_IMPLICIT, false },      /* ProtectKernelTunables= */
         { "/dev",                MOUNT_READ_WRITE_IMPLICIT, false },      /* PrivateDevices= */
@@ -243,7 +242,6 @@ static const char * const mount_mode_table[_MOUNT_MODE_MAX] = {
         [MOUNT_BIND]                  = "bind",
         [MOUNT_BIND_RECURSIVE]        = "bind-recursive",
         [MOUNT_PRIVATE_TMP]           = "private-tmp",
-        [MOUNT_PRIVATE_TMP_READ_ONLY] = "private-tmp-read-only",
         [MOUNT_PRIVATE_DEV]           = "private-dev",
         [MOUNT_BIND_DEV]              = "bind-dev",
         [MOUNT_EMPTY_DIR]             = "empty-dir",
@@ -280,7 +278,7 @@ static const struct {
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(mount_mode, MountMode);
 
-static const char *mount_entry_path(const MountEntry *p) {
+static const char* mount_entry_path(const MountEntry *p) {
         assert(p);
 
         /* Returns the path of this bind mount. If the malloc()-allocated ->path_buffer field is set we return that,
@@ -289,7 +287,7 @@ static const char *mount_entry_path(const MountEntry *p) {
         return p->path_malloc ?: p->path_const;
 }
 
-static const char *mount_entry_unprefixed_path(const MountEntry *p) {
+static const char* mount_entry_unprefixed_path(const MountEntry *p) {
         assert(p);
 
         /* Returns the unprefixed path (ie: before prefix_where_needed() ran), if any */
@@ -315,7 +313,7 @@ static void mount_entry_consume_prefix(MountEntry *p, char *new_path) {
 static bool mount_entry_read_only(const MountEntry *p) {
         assert(p);
 
-        return p->read_only || IN_SET(p->mode, MOUNT_READ_ONLY, MOUNT_INACCESSIBLE, MOUNT_PRIVATE_TMP_READ_ONLY);
+        return p->read_only || IN_SET(p->mode, MOUNT_READ_ONLY, MOUNT_INACCESSIBLE);
 }
 
 static bool mount_entry_noexec(const MountEntry *p) {
@@ -330,13 +328,13 @@ static bool mount_entry_exec(const MountEntry *p) {
         return p->exec || p->mode == MOUNT_EXEC;
 }
 
-static const char *mount_entry_source(const MountEntry *p) {
+static const char* mount_entry_source(const MountEntry *p) {
         assert(p);
 
         return p->source_malloc ?: p->source_const;
 }
 
-static const char *mount_entry_options(const MountEntry *p) {
+static const char* mount_entry_options(const MountEntry *p) {
         assert(p);
 
         return p->options_malloc ?: p->options_const;
@@ -363,7 +361,7 @@ static void mount_list_done(MountList *ml) {
         ml->n_mounts = 0;
 }
 
-static MountEntry *mount_list_extend(MountList *ml) {
+static MountEntry* mount_list_extend(MountList *ml) {
         assert(ml);
 
         if (!GREEDY_REALLOC0(ml->mounts, ml->n_mounts+1))
@@ -1746,7 +1744,6 @@ static int apply_one_mount(
                 return mount_tmpfs(m);
 
         case MOUNT_PRIVATE_TMP:
-        case MOUNT_PRIVATE_TMP_READ_ONLY:
                 what = mount_entry_source(m);
                 make = true;
                 break;
@@ -2402,29 +2399,27 @@ int setup_namespace(const NamespaceParameters *p, char **error_path) {
                 assert(p->private_tmp == PRIVATE_TMP_CONNECTED);
 
                 if (p->tmp_dir) {
-                        bool ro = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY);
-
                         MountEntry *me = mount_list_extend(&ml);
                         if (!me)
                                 return log_oom_debug();
 
                         *me = (MountEntry) {
                                 .path_const = "/tmp",
-                                .mode = ro ? MOUNT_PRIVATE_TMP_READ_ONLY : MOUNT_PRIVATE_TMP,
+                                .mode = MOUNT_PRIVATE_TMP,
+                                .read_only = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY),
                                 .source_const = p->tmp_dir,
                         };
                 }
 
                 if (p->var_tmp_dir) {
-                        bool ro = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY);
-
                         MountEntry *me = mount_list_extend(&ml);
                         if (!me)
                                 return log_oom_debug();
 
                         *me = (MountEntry) {
                                 .path_const = "/var/tmp",
-                                .mode = ro ? MOUNT_PRIVATE_TMP_READ_ONLY : MOUNT_PRIVATE_TMP,
+                                .mode = MOUNT_PRIVATE_TMP,
+                                .read_only = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY),
                                 .source_const = p->var_tmp_dir,
                         };
                 }

src/test/test-execute.c

@@ -757,6 +757,10 @@ static int find_libraries(const char *exec, char ***ret) {
 #endif
 
 static void test_exec_mount_apivfs(Manager *m) {
+        test(m, "exec-mount-apivfs-yes.service",
+             MANAGER_IS_SYSTEM(m) || VIRTUALIZATION_IS_CONTAINER(detect_container()) ? 0 : EXIT_NAMESPACE,
+             CLD_EXITED);
+
 #if !HAS_FEATURE_ADDRESS_SANITIZER
         _cleanup_free_ char *fullpath_touch = NULL, *fullpath_test = NULL, *data = NULL;
         _cleanup_strv_free_ char **libraries = NULL, **libraries_test = NULL;

test/test-execute/exec-mount-apivfs-yes.service

@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for mount attributes for MountAPIVFS=yes
+
+[Service]
+Type=oneshot
+PrivateMounts=true
+MountAPIVFS=yes
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /proc --noheadings -o FSTYPE)" == proc ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /dev --noheadings -o FSTYPE)" == devtmpfs ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /sys --noheadings -o FSTYPE)" == sysfs ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /run --noheadings -o FSTYPE)" == tmpfs ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ rw ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ nosuid ]]'
+ExecStart=bash -x -c '[[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ nodev ]]'

test/test-execute/exec-protecthome-tmpfs-vs-protectsystem-strict.service

@@ -8,3 +8,11 @@ ProtectHome=tmpfs
 ProtectSystem=strict
 Type=oneshot
 ExecStart=sh -x -c 'test "$$(stat -fc %%T /home)" = "tmpfs"'
+ExecStart=sh -x -c '! touch /home/hoge'
+ExecStart=sh -x -c '! touch /run/user/hoge'
+ExecStart=sh -x -c '! touch /root/hoge'
+
+# Even if /home is read-only, the submount should be writable.
+TemporaryFileSystem=/home/foo
+ExecStart=sh -x -c 'test "$$(stat -fc %%T /home/foo)" = "tmpfs"'
+ExecStart=sh -x -c 'touch /home/foo/hoge'