From: Yu Watanabe Date: Thu, 1 May 2025 11:58:18 +0000 (+0900) Subject: core: disable mounting disconnected private tmpfs on /var/tmp/ when DefaultDependenci... X-Git-Tag: v258-rc1~645^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6156bec7a464815084fa5218fe782ea6cb20ad52;p=thirdparty%2Fsystemd.git core: disable mounting disconnected private tmpfs on /var/tmp/ when DefaultDependencies=no 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 --- diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index b0bf5025767..77fc647f7b6 100644 --- 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 0b715126536..18ec06b6a49 100644 --- 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 4aef0ad8198..417d0be5756 100644 --- 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 c8e78bee24c..756d78117c0 100644 --- 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 67437549eaf..48ddea10d7d 100644 --- 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, ¶ms, &runtime, &cgroup_context); diff --git a/src/core/namespace.c b/src/core/namespace.c index a80c0eb60f2..d80fe74afbf 100644 --- 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 0ff9b4d7309..f1076d0ee45 100644 --- 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 9d3988a635a..7e3eaa4c92f 100644 --- 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 8ba3a888ecd..8f3c7b41d83 100644 --- 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 337243972c2..1b1f1762c9d 100644 --- 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 31fdd7f0e2b..522bb010e7c 100644 --- 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 63552013b29..d0e448daa10 100644 --- 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");