core: disable mounting disconnected private tmpfs on /var/tmp/ when DefaultDependenci...

If DefaultDependencies=no, /var/ may not be mounted yet when the service
is being started. Previously, In such case, if the service has
PrivateTmp=disconnected, the service manager created /var/tmp/ on the
root filesystem and mounted the disconnected private tmpfs there. That
poluted the root filesystem and disturbed gpt-auto-generator on next
boot, as /var/ would not be empty anymore. See issue #37258.

This changes PrivateTmp=disconnected as the following:
- If DefaultDependencies=no and RootDirectory=/RootImage= are not set,
  then a private tmpfs is mounted _only_ on /tmp/, and set $TMPDIR=/tmp
  environment variable to suggest the service to use /tmp/.
- If DefaultDependencies=yes and RootDirectory=/RootImage= are not set,
  then implies RequiresMountsFor=/var/, though that is typically
  redundant, but anyway. Hence, we can safely mount /var/tmp/.
- Otherwise, i.e. when one of RootDirectory=/RootImage= is set, behaves
  as the same as the previous, as the private root filesystem for the
  service is explicitly prepared by the service manager, and we can
  safely mount a private tmpfs on /var/tmp/ without any extra
  dependencies.

Fixes #37258.

Co-authored-by: Mike Yuan <me@yhndnzj.com>

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index b0bf50257679d52d636f56771a3d9f5e7a82c249..77fc647f7b6923a632267b77efc9707a898f080e 100644 (file)
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -1921,7 +1921,7 @@ static int build_environment(
         assert(cgroup_context);
         assert(ret);
 
-#define N_ENV_VARS 21
+#define N_ENV_VARS 22
         our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX);
         if (!our_env)
                 return -ENOMEM;
@@ -2181,6 +2181,21 @@ static int build_environment(
                 our_env[n_env++] = x;
         }
 
+        assert(c->private_var_tmp >= 0 && c->private_var_tmp < _PRIVATE_TMP_MAX);
+        if (c->private_tmp != c->private_var_tmp) {
+                assert(c->private_tmp == PRIVATE_TMP_DISCONNECTED);
+                assert(c->private_var_tmp == PRIVATE_TMP_NO);
+
+                /* When private tmpfs is enabled only on /tmp/, then explicitly set $TMPDIR to suggest the
+                 * service to use /tmp/. */
+
+                x = strdup("TMPDIR=/tmp");
+                if (!x)
+                        return -ENOMEM;
+
+                our_env[n_env++] = x;
+        }
+
         assert(n_env < N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX);
 #undef N_ENV_VARS
 
@@ -3589,6 +3604,7 @@ static int apply_mount_namespace(
                 .private_ipc = needs_sandboxing && exec_needs_ipc_namespace(context),
                 .private_pids = needs_sandboxing && exec_needs_pid_namespace(context) ? context->private_pids : PRIVATE_PIDS_NO,
                 .private_tmp = needs_sandboxing ? context->private_tmp : PRIVATE_TMP_NO,
+                .private_var_tmp = needs_sandboxing ? context->private_var_tmp : PRIVATE_TMP_NO,
 
                 .mount_apivfs = needs_sandboxing && exec_context_get_effective_mount_apivfs(context),
                 .bind_log_sockets = needs_sandboxing && exec_context_get_effective_bind_log_sockets(context),

diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c
index 0b715126536dba39eebb02ecfc2de5ecea597deb..18ec06b6a49f8c8f7635a0fa634fb0835502cca6 100644 (file)
--- a/src/core/execute-serialize.c
+++ b/src/core/execute-serialize.c
@@ -1735,6 +1735,12 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) {
         if (r < 0)
                 return r;
 
+        /* This must be set in unit_patch_contexts() before executing a command. */
+        assert(c->private_var_tmp >= 0 && c->private_var_tmp < _PRIVATE_TMP_MAX);
+        r = serialize_item(f, "exec-context-private-var-tmp", private_tmp_to_string(c->private_var_tmp));
+        if (r < 0)
+                return r;
+
         r = serialize_bool_elide(f, "exec-context-private-devices", c->private_devices);
         if (r < 0)
                 return r;
@@ -2628,6 +2634,10 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) {
                         c->private_tmp = private_tmp_from_string(val);
                         if (c->private_tmp < 0)
                                 return c->private_tmp;
+                } else if ((val = startswith(l, "exec-context-private-var-tmp="))) {
+                        c->private_var_tmp = private_tmp_from_string(val);
+                        if (c->private_var_tmp < 0)
+                                return c->private_var_tmp;
                 } else if ((val = startswith(l, "exec-context-private-devices="))) {
                         r = parse_boolean(val);
                         if (r < 0)

diff --git a/src/core/execute.c b/src/core/execute.c
index 4aef0ad81988fea35ae787e6b29376a8be519695..417d0be57564bb8d6bbf9e56817a29b89abd7709 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -635,6 +635,7 @@ void exec_context_init(ExecContext *c) {
                 .mount_apivfs = -1,
                 .bind_log_sockets = -1,
                 .memory_ksm = -1,
+                .private_var_tmp = _PRIVATE_TMP_INVALID,
                 .set_login_environment = -1,
         };
 

diff --git a/src/core/execute.h b/src/core/execute.h
index c8e78bee24ca2169b5a3544cc45112364000c229..756d78117c0cb9acb351e0f469d680bc160960f6 100644 (file)
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -330,6 +330,8 @@ struct ExecContext {
         int bind_log_sockets;
         int memory_ksm;
         PrivateTmp private_tmp;
+        PrivateTmp private_var_tmp; /* This is not an independent parameter, but calculated from other
+                                     * parameters in unit_patch_contexts(). */
         bool private_network;
         bool private_devices;
         PrivateUsers private_users;

diff --git a/src/core/fuzz-execute-serialize.c b/src/core/fuzz-execute-serialize.c
index 67437549eaf62b0ce757e0e1710cfdf08281d999..48ddea10d7d56acb009d79494d69061b64104164 100644 (file)
--- a/src/core/fuzz-execute-serialize.c
+++ b/src/core/fuzz-execute-serialize.c
@@ -43,6 +43,7 @@ static void exec_fuzz_one(FILE *f, FDSet *fdset) {
         };
 
         exec_context_init(&exec_context);
+        exec_context.private_var_tmp = PRIVATE_TMP_DISCONNECTED;
         cgroup_context_init(&cgroup_context);
 
         (void) exec_deserialize_invocation(f, fdset, &exec_context, &command, &params, &runtime, &cgroup_context);

diff --git a/src/core/namespace.c b/src/core/namespace.c
index a80c0eb60f2619781886cbb96ec520ee7c682ceb..d80fe74afbfb624cf7b3a6c7647b14af1d75a22d 100644 (file)
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -736,6 +736,8 @@ static int append_private_tmp(MountList *ml, const NamespaceParameters *p) {
 
         assert(ml);
         assert(p);
+        assert(p->private_tmp == p->private_var_tmp ||
+               (p->private_tmp == PRIVATE_TMP_DISCONNECTED && p->private_var_tmp == PRIVATE_TMP_NO));
 
         if (p->tmp_dir) {
                 assert(p->private_tmp == PRIVATE_TMP_CONNECTED);
@@ -752,7 +754,7 @@ static int append_private_tmp(MountList *ml, const NamespaceParameters *p) {
         }
 
         if (p->var_tmp_dir) {
-                assert(p->private_tmp == PRIVATE_TMP_CONNECTED);
+                assert(p->private_var_tmp == PRIVATE_TMP_CONNECTED);
 
                 me = mount_list_extend(ml);
                 if (!me)
@@ -768,6 +770,20 @@ static int append_private_tmp(MountList *ml, const NamespaceParameters *p) {
         if (p->private_tmp != PRIVATE_TMP_DISCONNECTED)
                 return 0;
 
+        if (p->private_var_tmp == PRIVATE_TMP_NO) {
+                me = mount_list_extend(ml);
+                if (!me)
+                        return log_oom_debug();
+                *me = (MountEntry) {
+                        .path_const = "/tmp/",
+                        .mode = MOUNT_PRIVATE_TMPFS,
+                        .options_const = "mode=0700" NESTED_TMPFS_LIMITS,
+                        .flags = MS_NODEV|MS_STRICTATIME,
+                };
+
+                return 0;
+        }
+
         _cleanup_free_ char *tmpfs_dir = NULL, *tmp_dir = NULL, *var_tmp_dir = NULL;
         tmpfs_dir = path_join(p->private_namespace_dir, "unit-private-tmp");
         tmp_dir = path_join(tmpfs_dir, "tmp");

diff --git a/src/core/namespace.h b/src/core/namespace.h
index 0ff9b4d730962e2facc24140d8a9b624a970d756..f1076d0ee45441c7ef6bc0dfd4708ae22ddad96b 100644 (file)
--- a/src/core/namespace.h
+++ b/src/core/namespace.h
@@ -199,6 +199,7 @@ struct NamespaceParameters {
         ProtectProc protect_proc;
         ProcSubset proc_subset;
         PrivateTmp private_tmp;
+        PrivateTmp private_var_tmp;
         PrivatePIDs private_pids;
 };
 

diff --git a/src/core/unit.c b/src/core/unit.c
index 9d3988a635a7e4a8c996beac2febc7f59eaea7a7..7e3eaa4c92f5545d90af73c3bc79da75f1660faf 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1270,7 +1270,12 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) {
                         return r;
         }
 
+        /* This must be already set in unit_patch_contexts(). */
+        assert(c->private_var_tmp >= 0 && c->private_var_tmp < _PRIVATE_TMP_MAX);
+
         if (c->private_tmp == PRIVATE_TMP_CONNECTED) {
+                assert(c->private_var_tmp == PRIVATE_TMP_CONNECTED);
+
                 r = unit_add_mounts_for(u, "/tmp/", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS);
                 if (r < 0)
                         return r;
@@ -1282,6 +1287,13 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) {
                 r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_TMPFILES_SETUP_SERVICE, true, UNIT_DEPENDENCY_FILE);
                 if (r < 0)
                         return r;
+
+        } else if (c->private_var_tmp == PRIVATE_TMP_DISCONNECTED && !exec_context_with_rootfs(c)) {
+                /* Even if PrivateTmp=disconnected, we still require /var/tmp/ mountpoint to be present,
+                 * i.e. /var/ needs to be mounted. See comments in unit_patch_contexts(). */
+                r = unit_add_mounts_for(u, "/var/", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS);
+                if (r < 0)
+                        return r;
         }
 
         if (c->root_image) {
@@ -4250,6 +4262,51 @@ static int unit_verify_contexts(const Unit *u) {
         return 0;
 }
 
+static PrivateTmp unit_get_private_var_tmp(const Unit *u, const ExecContext *c) {
+        assert(u);
+        assert(c);
+        assert(c->private_tmp >= 0 && c->private_tmp < _PRIVATE_TMP_MAX);
+
+        /* Disable disconnected private tmpfs on /var/tmp/ when DefaultDependencies=no and
+         * RootImage=/RootDirectory= are not set, as /var/ may be a separated partition.
+         * See issue #37258. */
+
+        /* PrivateTmp=yes/no also enables/disables private tmpfs on /var/tmp/. */
+        if (c->private_tmp != PRIVATE_TMP_DISCONNECTED)
+                return c->private_tmp;
+
+        /* When DefaultDependencies=yes, disconnected tmpfs is also enabled on /var/tmp/, and an explicit
+         * dependency to the mount on /var/ will be added in unit_add_exec_dependencies(). */
+        if (u->default_dependencies)
+                return PRIVATE_TMP_DISCONNECTED;
+
+        /* When RootImage=/RootDirectory= is enabled, /var/ should be prepared by the image or directory,
+         * hence we can mount a disconnected tmpfs on /var/tmp/. */
+        if (exec_context_with_rootfs(c))
+                return PRIVATE_TMP_DISCONNECTED;
+
+        /* Even if DefaultDependencies=no, enable disconnected tmpfs when
+         * RequiresMountsFor=/WantsMountsFor=/var/ is explicitly set. */
+        for (UnitMountDependencyType t = 0; t < _UNIT_MOUNT_DEPENDENCY_TYPE_MAX; t++)
+                if (hashmap_contains(u->mounts_for[t], "/var/"))
+                        return PRIVATE_TMP_DISCONNECTED;
+
+        /* Check the same but for After= with Requires=/Requisite=/Wants= or friends. */
+        Unit *m = manager_get_unit(u->manager, "var.mount");
+        if (!m)
+                return PRIVATE_TMP_NO;
+
+        if (!unit_has_dependency(u, UNIT_ATOM_AFTER, m))
+                return PRIVATE_TMP_NO;
+
+        if (unit_has_dependency(u, UNIT_ATOM_PULL_IN_START, m) ||
+            unit_has_dependency(u, UNIT_ATOM_PULL_IN_VERIFY, m) ||
+            unit_has_dependency(u, UNIT_ATOM_PULL_IN_START_IGNORED, m))
+                return PRIVATE_TMP_DISCONNECTED;
+
+        return PRIVATE_TMP_NO;
+}
+
 int unit_patch_contexts(Unit *u) {
         CGroupContext *cc;
         ExecContext *ec;
@@ -4326,6 +4383,8 @@ int unit_patch_contexts(Unit *u) {
                         ec->restrict_suid_sgid = true;
                 }
 
+                ec->private_var_tmp = unit_get_private_var_tmp(u, ec);
+
                 FOREACH_ARRAY(d, ec->directories, _EXEC_DIRECTORY_TYPE_MAX)
                         exec_directory_sort(d);
         }

diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c
index 8ba3a888ecd1af79de694a5bd8e9d63578d64c7f..8f3c7b41d83deb2f0e9077d54118060115a0944d 100644 (file)
--- a/src/test/test-bpf-firewall.c
+++ b/src/test/test-bpf-firewall.c
@@ -168,6 +168,7 @@ int main(int argc, char *argv[]) {
 
         ASSERT_OK(r);
 
+        ASSERT_OK(unit_patch_contexts(u));
         ASSERT_OK(unit_start(u, NULL));
 
         while (!IN_SET(SERVICE(u)->state, SERVICE_DEAD, SERVICE_FAILED))
@@ -193,6 +194,7 @@ int main(int argc, char *argv[]) {
         SERVICE(u)->type = SERVICE_ONESHOT;
         u->load_state = UNIT_LOADED;
 
+        ASSERT_OK(unit_patch_contexts(u));
         ASSERT_OK(unit_start(u, NULL));
 
         while (!IN_SET(SERVICE(u)->state, SERVICE_DEAD, SERVICE_FAILED))

diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c
index 337243972c2f9164309ff0fe3cee2761acf35704..1b1f1762c9dee1b0f2d9052660bee59490e51233 100644 (file)
--- a/src/test/test-bpf-foreign-programs.c
+++ b/src/test/test-bpf-foreign-programs.c
@@ -246,6 +246,7 @@ static int test_bpf_cgroup_programs(Manager *m, const char *unit_name, const Tes
         SERVICE(u)->type = SERVICE_ONESHOT;
         u->load_state = UNIT_LOADED;
 
+        ASSERT_OK(unit_patch_contexts(u));
         r = unit_start(u, NULL);
         if (r < 0)
                 return log_error_errno(r, "Unit start failed %m");

diff --git a/src/test/test-bpf-restrict-fs.c b/src/test/test-bpf-restrict-fs.c
index 31fdd7f0e2b2a9fa55ff6b2536803a2db26cb6dd..522bb010e7cf3f2e3dc5b6af5eb03361082d7c96 100644 (file)
--- a/src/test/test-bpf-restrict-fs.c
+++ b/src/test/test-bpf-restrict-fs.c
@@ -39,6 +39,7 @@ static int test_restrict_filesystems(Manager *m, const char *unit_name, const ch
         SERVICE(u)->type = SERVICE_ONESHOT;
         u->load_state = UNIT_LOADED;
 
+        ASSERT_OK(unit_patch_contexts(u));
         r = unit_start(u, NULL);
         if (r < 0)
                 return log_error_errno(r, "Unit start failed %m");

diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c
index 63552013b29a7f1cf5d4bfd50682aa3559189ecd..d0e448daa10e13fd7dfab7246530ea531d2ffb17 100644 (file)
--- a/src/test/test-socket-bind.c
+++ b/src/test/test-socket-bind.c
@@ -76,6 +76,7 @@ static int test_socket_bind(
         SERVICE(u)->type = SERVICE_ONESHOT;
         u->load_state = UNIT_LOADED;
 
+        ASSERT_OK(unit_patch_contexts(u));
         r = unit_start(u, NULL);
         if (r < 0)
                 return log_error_errno(r, "Unit start failed %m");