From 813c96c0590a8855fb45eab8665f91533df295a7 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 26 Apr 2025 16:09:48 +0200 Subject: [PATCH 1/9] core/unit: clean up duplicate conditions --- src/core/unit.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index c706caae08..d242bb7196 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1293,18 +1293,8 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { return r; } - if (!IN_SET(c->std_output, - EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE, - EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE) && - !IN_SET(c->std_error, - EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE, - EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE) && - !c->log_namespace) - return 0; - /* If syslog or kernel logging is requested (or log namespacing is), make sure our own logging daemon * is run first. */ - if (c->log_namespace) { static const struct { const char *template; @@ -1326,7 +1316,11 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { if (r < 0) return r; } - } else { + } else if (IN_SET(c->std_output, EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE, + EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE) || + IN_SET(c->std_error, EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE, + EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE)) { + r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_JOURNALD_SOCKET, true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; From a4b1232dbf604fbd357cb4a9ef2024a703407131 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 May 2025 19:44:07 +0900 Subject: [PATCH 2/9] core/unit: suffix directory paths with slash --- src/core/unit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index d242bb7196..9d3988a635 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1271,11 +1271,11 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { } if (c->private_tmp == PRIVATE_TMP_CONNECTED) { - r = unit_add_mounts_for(u, "/tmp", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS); + r = unit_add_mounts_for(u, "/tmp/", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS); if (r < 0) return r; - r = unit_add_mounts_for(u, "/var/tmp", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS); + r = unit_add_mounts_for(u, "/var/tmp/", UNIT_DEPENDENCY_FILE, UNIT_MOUNT_WANTS); if (r < 0) return r; From 97bf617af153898ebb7dc21a644f2ff34d5420eb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 28 Apr 2025 14:45:04 +0900 Subject: [PATCH 3/9] core/namespace: introduce should_propagate_to_submounts() helper function --- src/core/namespace.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 381de7528a..b1bcdb18b7 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1907,6 +1907,11 @@ static int apply_one_mount( return 1; } +static bool should_propagate_to_submounts(const MountEntry *m) { + assert(m); + return !IN_SET(m->mode, MOUNT_EMPTY_DIR, MOUNT_TMPFS, MOUNT_PRIVATE_TMPFS); +} + static int make_read_only(const MountEntry *m, char **deny_list, FILE *proc_self_mountinfo) { unsigned long new_flags = 0, flags_mask = 0; bool submounts; @@ -1935,9 +1940,7 @@ static int make_read_only(const MountEntry *m, char **deny_list, FILE *proc_self * nothing further down. Set /dev readonly, but not submounts like /dev/shm. Also, we only set the * per-mount read-only flag. We can't set it on the superblock, if we are inside a user namespace * and running Linux <= 4.17. */ - submounts = - mount_entry_read_only(m) && - !IN_SET(m->mode, MOUNT_EMPTY_DIR, MOUNT_TMPFS, MOUNT_PRIVATE_TMPFS); + submounts = mount_entry_read_only(m) && should_propagate_to_submounts(m); if (submounts) r = bind_remount_recursive_with_mountinfo(mount_entry_path(m), new_flags, flags_mask, deny_list, proc_self_mountinfo); else @@ -1977,8 +1980,7 @@ static int make_noexec(const MountEntry *m, char **deny_list, FILE *proc_self_mo if (flags_mask == 0) /* No Change? */ return 0; - submounts = !IN_SET(m->mode, MOUNT_EMPTY_DIR, MOUNT_TMPFS, MOUNT_PRIVATE_TMPFS); - + submounts = should_propagate_to_submounts(m); if (submounts) r = bind_remount_recursive_with_mountinfo(mount_entry_path(m), new_flags, flags_mask, deny_list, proc_self_mountinfo); else @@ -2002,7 +2004,7 @@ static int make_nosuid(const MountEntry *m, FILE *proc_self_mountinfo) { if (m->state != MOUNT_APPLIED) return 0; - submounts = !IN_SET(m->mode, MOUNT_EMPTY_DIR, MOUNT_TMPFS, MOUNT_PRIVATE_TMPFS); + submounts = should_propagate_to_submounts(m); if (submounts) r = bind_remount_recursive_with_mountinfo(mount_entry_path(m), MS_NOSUID, MS_NOSUID, NULL, proc_self_mountinfo); else From 13208591db78133907c41f01546123d132336276 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 May 2025 20:06:48 +0900 Subject: [PATCH 4/9] core/namespace: split out append_private_tmp() --- src/core/namespace.c | 157 +++++++++++++++++++++++-------------------- 1 file changed, 83 insertions(+), 74 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index b1bcdb18b7..a80c0eb60f 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -731,6 +731,86 @@ static int append_tmpfs_mounts(MountList *ml, const TemporaryFileSystem *tmpfs, return 0; } +static int append_private_tmp(MountList *ml, const NamespaceParameters *p) { + MountEntry *me; + + assert(ml); + assert(p); + + if (p->tmp_dir) { + assert(p->private_tmp == PRIVATE_TMP_CONNECTED); + + me = mount_list_extend(ml); + if (!me) + return log_oom_debug(); + *me = (MountEntry) { + .path_const = "/tmp/", + .mode = MOUNT_PRIVATE_TMP, + .read_only = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY), + .source_const = p->tmp_dir, + }; + } + + if (p->var_tmp_dir) { + assert(p->private_tmp == PRIVATE_TMP_CONNECTED); + + me = mount_list_extend(ml); + if (!me) + return log_oom_debug(); + *me = (MountEntry) { + .path_const = "/var/tmp/", + .mode = MOUNT_PRIVATE_TMP, + .read_only = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY), + .source_const = p->var_tmp_dir, + }; + } + + if (p->private_tmp != PRIVATE_TMP_DISCONNECTED) + 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"); + var_tmp_dir = path_join(tmpfs_dir, "var-tmp"); + if (!tmpfs_dir || !tmp_dir || !var_tmp_dir) + return log_oom_debug(); + + me = mount_list_extend(ml); + if (!me) + return log_oom_debug(); + *me = (MountEntry) { + .path_malloc = TAKE_PTR(tmpfs_dir), + .mode = MOUNT_PRIVATE_TMPFS, + .options_const = "mode=0700" NESTED_TMPFS_LIMITS, + .flags = MS_NODEV|MS_STRICTATIME, + .has_prefix = true, + }; + + me = mount_list_extend(ml); + if (!me) + return log_oom_debug(); + *me = (MountEntry) { + .source_malloc = TAKE_PTR(tmp_dir), + .path_const = "/tmp/", + .mode = MOUNT_BIND, + .source_dir_mode = 01777, + .create_source_dir = true, + }; + + me = mount_list_extend(ml); + if (!me) + return log_oom_debug(); + *me = (MountEntry) { + .source_malloc = TAKE_PTR(var_tmp_dir), + .path_const = "/var/tmp/", + .mode = MOUNT_BIND, + .source_dir_mode = 01777, + .create_source_dir = true, + }; + + return 0; +} + static int append_static_mounts(MountList *ml, const MountEntry *mounts, size_t n, bool ignore_protect) { assert(ml); assert(mounts || n == 0); @@ -2452,80 +2532,9 @@ int setup_namespace(const NamespaceParameters *p, char **reterr_path) { if (r < 0) return r; - /* When DynamicUser=yes enforce that /tmp/ and /var/tmp/ are disconnected from the host's - * directories, as they are world writable and ephemeral uid/gid will be used. */ - if (p->private_tmp == PRIVATE_TMP_DISCONNECTED) { - _cleanup_free_ char *tmpfs_dir = NULL, *tmp_dir = NULL, *var_tmp_dir = NULL; - MountEntry *tmpfs_entry, *tmp_entry, *var_tmp_entry; - - tmpfs_dir = path_join(p->private_namespace_dir, "unit-private-tmp"); - tmp_dir = path_join(tmpfs_dir, "tmp"); - var_tmp_dir = path_join(tmpfs_dir, "var-tmp"); - if (!tmpfs_dir || !tmp_dir || !var_tmp_dir) - return log_oom_debug(); - - tmpfs_entry = mount_list_extend(&ml); - if (!tmpfs_entry) - return log_oom_debug(); - *tmpfs_entry = (MountEntry) { - .path_malloc = TAKE_PTR(tmpfs_dir), - .mode = MOUNT_PRIVATE_TMPFS, - .options_const = "mode=0700" NESTED_TMPFS_LIMITS, - .flags = MS_NODEV|MS_STRICTATIME, - .has_prefix = true, - }; - - tmp_entry = mount_list_extend(&ml); - if (!tmp_entry) - return log_oom_debug(); - *tmp_entry = (MountEntry) { - .source_malloc = TAKE_PTR(tmp_dir), - .path_const = "/tmp", - .mode = MOUNT_BIND, - .source_dir_mode = 01777, - .create_source_dir = true, - }; - - var_tmp_entry = mount_list_extend(&ml); - if (!var_tmp_entry) - return log_oom_debug(); - *var_tmp_entry = (MountEntry) { - .source_malloc = TAKE_PTR(var_tmp_dir), - .path_const = "/var/tmp", - .mode = MOUNT_BIND, - .source_dir_mode = 01777, - .create_source_dir = true, - }; - - } else if (p->tmp_dir || p->var_tmp_dir) { - assert(p->private_tmp == PRIVATE_TMP_CONNECTED); - - if (p->tmp_dir) { - MountEntry *me = mount_list_extend(&ml); - if (!me) - return log_oom_debug(); - - *me = (MountEntry) { - .path_const = "/tmp", - .mode = MOUNT_PRIVATE_TMP, - .read_only = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY), - .source_const = p->tmp_dir, - }; - } - - if (p->var_tmp_dir) { - MountEntry *me = mount_list_extend(&ml); - if (!me) - return log_oom_debug(); - - *me = (MountEntry) { - .path_const = "/var/tmp", - .mode = MOUNT_PRIVATE_TMP, - .read_only = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY), - .source_const = p->var_tmp_dir, - }; - } - } + r = append_private_tmp(&ml, p); + if (r < 0) + return r; r = append_mount_images(&ml, p->mount_images, p->n_mount_images); if (r < 0) From fee2f5ca3fc64c343806323548360dbca38cbcea Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 May 2025 16:11:37 +0900 Subject: [PATCH 5/9] core/mount,swap: reload unit when manually mounted/started and we previously failed to load Otherwise, neither unit_patch_contexts() nor unit_add_exec_dependencies() is called for the unit. --- src/core/mount.c | 6 ++++-- src/core/swap.c | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index c496375a21..2541a4888c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1855,9 +1855,11 @@ static int mount_setup_existing_unit( if (UNIT_IS_LOAD_ERROR(u->load_state)) { /* The unit was previously not found or otherwise not loaded. Now that the unit shows up in - * /proc/self/mountinfo we should reconsider it this, hence set it to UNIT_LOADED. */ - u->load_state = UNIT_LOADED; + * /proc/self/mountinfo we should reconsider that. Hence, let's reset the load state and load + * error, and add the unit to load queue. */ + u->load_state = UNIT_STUB; u->load_error = 0; + unit_add_to_load_queue(u); flags |= MOUNT_PROC_JUST_CHANGED; } diff --git a/src/core/swap.c b/src/core/swap.c index 6d505c2acf..668e1bcf6c 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -406,8 +406,6 @@ static int swap_setup_unit( s->what = strdup(what); if (!s->what) return log_oom(); - - unit_add_to_load_queue(u); } SwapParameters *p = &s->parameters_proc_swaps; @@ -418,13 +416,15 @@ static int swap_setup_unit( return log_oom(); } - /* The unit is definitely around now, mark it as loaded if it was previously referenced but - * could not be loaded. After all we can load it now, from the data in /proc/swaps. */ + /* The unit is definitely around now. When we previously fail to load the unit, let's reload the unit + * by resetting the load state and load error, and adding the unit to the load queue. */ if (UNIT_IS_LOAD_ERROR(u->load_state)) { - u->load_state = UNIT_LOADED; + u->load_state = UNIT_STUB; u->load_error = 0; } + unit_add_to_load_queue(u); + if (set_flags) { s->is_active = true; s->just_activated = !s->from_proc_swaps; From 26186503d71837de829c49b29ae29529bf447097 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 11 May 2025 03:22:04 +0900 Subject: [PATCH 6/9] core/mount: drop unnecessary dependency generations When the unit is new, then mount_setup_new_unit() adds the unit to the load queue, and the same dependencies will be anyway added. When the unit already exists but previously failed to be loaded, then mount_setup_existing_unit() also adds the unit to the load queue. Hence it is not necessary to regenerate dependencies here now. So, we need to regenerate dependencies only when things changed and the unit has been already loaded. No functional changes, just optimization. --- src/core/mount.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 2541a4888c..d7211de024 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1795,10 +1795,6 @@ static int mount_setup_new_unit( * attributes the deps to. */ mnt->from_proc_self_mountinfo = true; - r = mount_add_non_exec_dependencies(mnt); - if (r < 0) - return r; - /* We have only allocated the stub now, let's enqueue this unit for loading now, so that everything * else is loaded in now. */ unit_add_to_load_queue(u); @@ -1864,9 +1860,9 @@ static int mount_setup_existing_unit( flags |= MOUNT_PROC_JUST_CHANGED; } - if (FLAGS_SET(flags, MOUNT_PROC_JUST_CHANGED)) { - /* If things changed, then make sure that all deps are regenerated. Let's - * first remove all automatic deps, and then add in the new ones. */ + if (FLAGS_SET(flags, MOUNT_PROC_JUST_CHANGED) && u->load_state == UNIT_LOADED) { + /* If things changed, and we have successfully loaded the unit, then make sure that all deps + * are regenerated. Let's first remove all automatic deps, and then add in the new ones. */ r = mount_add_non_exec_dependencies(m); if (r < 0) return r; From 6156bec7a464815084fa5218fe782ea6cb20ad52 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 May 2025 20:58:18 +0900 Subject: [PATCH 7/9] 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 --- src/core/exec-invoke.c | 18 ++++++++- src/core/execute-serialize.c | 10 +++++ src/core/execute.c | 1 + src/core/execute.h | 2 + src/core/fuzz-execute-serialize.c | 1 + src/core/namespace.c | 18 ++++++++- src/core/namespace.h | 1 + src/core/unit.c | 59 ++++++++++++++++++++++++++++ src/test/test-bpf-firewall.c | 2 + src/test/test-bpf-foreign-programs.c | 1 + src/test/test-bpf-restrict-fs.c | 1 + src/test/test-socket-bind.c | 1 + 12 files changed, 113 insertions(+), 2 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index b0bf502576..77fc647f7b 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 0b71512653..18ec06b6a4 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 4aef0ad819..417d0be575 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 c8e78bee24..756d78117c 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 67437549ea..48ddea10d7 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 a80c0eb60f..d80fe74afb 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 0ff9b4d730..f1076d0ee4 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 9d3988a635..7e3eaa4c92 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 8ba3a888ec..8f3c7b41d8 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 337243972c..1b1f1762c9 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 31fdd7f0e2..522bb010e7 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 63552013b2..d0e448daa1 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"); From 3e320dc608d3ff0325a217274cc34cae06e73452 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 28 Apr 2025 15:29:13 +0900 Subject: [PATCH 8/9] test-execute: add test case for PrivateTmp=disconnected --- src/test/test-execute.c | 14 ++++++++++++++ ...isconnected-after-and-requires-for-var.service | 15 +++++++++++++++ ...p-disconnected-after-and-wants-for-var.service | 15 +++++++++++++++ ...mp-disconnected-defaultdependencies-no.service | 13 +++++++++++++ ...p-disconnected-requires-mounts-for-var.service | 14 ++++++++++++++ ...etmp-disconnected-wants-mounts-for-var.service | 14 ++++++++++++++ .../exec-privatetmp-disconnected.service | 12 ++++++++++++ 7 files changed, 97 insertions(+) create mode 100644 test/test-execute/exec-privatetmp-disconnected-after-and-requires-for-var.service create mode 100644 test/test-execute/exec-privatetmp-disconnected-after-and-wants-for-var.service create mode 100644 test/test-execute/exec-privatetmp-disconnected-defaultdependencies-no.service create mode 100644 test/test-execute/exec-privatetmp-disconnected-requires-mounts-for-var.service create mode 100644 test/test-execute/exec-privatetmp-disconnected-wants-mounts-for-var.service create mode 100644 test/test-execute/exec-privatetmp-disconnected.service diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 5ba680d571..1eddf11dd1 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -491,6 +491,20 @@ static void test_exec_privatetmp(Manager *m) { if (MANAGER_IS_SYSTEM(m) || have_userns_privileges()) { test(m, "exec-privatetmp-yes.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED); test(m, "exec-privatetmp-disabled-by-prefix.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED); + + FOREACH_STRING(s, + "exec-privatetmp-disconnected.service", + "exec-privatetmp-disconnected-defaultdependencies-no.service", + "exec-privatetmp-disconnected-requires-mounts-for-var.service", + "exec-privatetmp-disconnected-wants-mounts-for-var.service", + "exec-privatetmp-disconnected-after-and-requires-for-var.service", + "exec-privatetmp-disconnected-after-and-wants-for-var.service") { + (void) unlink("/tmp/test-exec_privatetmp_disconnected"); + (void) unlink("/var/tmp/test-exec_privatetmp_disconnected"); + test(m, s, can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED); + ASSERT_FAIL(access("/tmp/test-exec_privatetmp_disconnected", F_OK)); + ASSERT_FAIL(access("/var/tmp/test-exec_privatetmp_disconnected", F_OK)); + } } test(m, "exec-privatetmp-no.service", 0, CLD_EXITED); diff --git a/test/test-execute/exec-privatetmp-disconnected-after-and-requires-for-var.service b/test/test-execute/exec-privatetmp-disconnected-after-and-requires-for-var.service new file mode 100644 index 0000000000..39e49c698a --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected-after-and-requires-for-var.service @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected with After=/Requires=var.mount +DefaultDependencies=no +After=var.mount +Requires=var.mount + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=touch /var/tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c 'test x$$TMPDIR = x' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected diff --git a/test/test-execute/exec-privatetmp-disconnected-after-and-wants-for-var.service b/test/test-execute/exec-privatetmp-disconnected-after-and-wants-for-var.service new file mode 100644 index 0000000000..b11302f5f2 --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected-after-and-wants-for-var.service @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected with After=/Wants=var.mount +DefaultDependencies=no +After=var.mount +Wants=var.mount + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=touch /var/tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c 'test x$$TMPDIR = x' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected diff --git a/test/test-execute/exec-privatetmp-disconnected-defaultdependencies-no.service b/test/test-execute/exec-privatetmp-disconnected-defaultdependencies-no.service new file mode 100644 index 0000000000..2208b8e1b5 --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected-defaultdependencies-no.service @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected with DefaultDependencies=no +DefaultDependencies=no + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c '(! touch /var/tmp/test-exec_privatetmp_disconnected)' +ExecStart=sh -x -c 'test $$TMPDIR = /tmp' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected diff --git a/test/test-execute/exec-privatetmp-disconnected-requires-mounts-for-var.service b/test/test-execute/exec-privatetmp-disconnected-requires-mounts-for-var.service new file mode 100644 index 0000000000..349c137984 --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected-requires-mounts-for-var.service @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected with RequiresMountsFor=/var/ +DefaultDependencies=no +RequiresMountsFor=/var/ + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=touch /var/tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c 'test x$$TMPDIR = x' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected diff --git a/test/test-execute/exec-privatetmp-disconnected-wants-mounts-for-var.service b/test/test-execute/exec-privatetmp-disconnected-wants-mounts-for-var.service new file mode 100644 index 0000000000..09ac3495b8 --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected-wants-mounts-for-var.service @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected with WantsMountsFor=/var/ +DefaultDependencies=no +WantsMountsFor=/var/ + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=touch /var/tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c 'test x$$TMPDIR = x' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected diff --git a/test/test-execute/exec-privatetmp-disconnected.service b/test/test-execute/exec-privatetmp-disconnected.service new file mode 100644 index 0000000000..fbee568d74 --- /dev/null +++ b/test/test-execute/exec-privatetmp-disconnected.service @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for PrivateTmp=disconnected + +[Service] +ExecStart=sh -x -c 'test ! -f /tmp/test-exec_privatetmp' +ExecStart=touch /tmp/test-exec_privatetmp_disconnected +ExecStart=touch /var/tmp/test-exec_privatetmp_disconnected +ExecStart=sh -x -c 'test x$$TMPDIR = x' +Type=oneshot +ProtectSystem=strict +PrivateTmp=disconnected From 8ac5b047fc86f2913adee89eb97ae7dc49bc95f8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 30 Apr 2025 02:27:16 +0900 Subject: [PATCH 9/9] man/systemd.exec: update documents for PrivateTmp= --- man/systemd.exec.xml | 122 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 100 insertions(+), 22 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index b350c19dcb..62c59308ef 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -61,13 +61,18 @@ paths. This is equivalent to having them listed explicitly in RequiresMountsFor=. - Similarly, units with PrivateTmp= enabled automatically get mount - unit dependencies for all mounts required to access /tmp/ and - /var/tmp/. They will also gain an automatic After= dependency - on + Units with PrivateTmp=yes automatically gain dependencies of type + Wants= and After= on all mounts required to access + /tmp/ and /var/tmp/. They will also gain an automatic + After= dependency on systemd-tmpfiles-setup.service8. + Units with PrivateTmp=disconnected automatically gain dependencies + of type Wants= and After= on the mount required to access + /var/, unless DefaultDependencies=no and/or + RootDirectory=/RootImage= are specified. + Units whose standard output or error output is connected to or (or their combinations with console output, see below) automatically acquire dependencies of type After= on @@ -1797,28 +1802,89 @@ BindReadOnlyPaths=/var/lib/systemd Takes a boolean argument, or disconnected. If enabled, a new file system namespace will be set up for the executed processes, and /tmp/ and /var/tmp/ directories inside it are not shared with processes outside of - the namespace, plus all temporary files created by a service in these directories will be removed after - the service is stopped. If true, the backing storage of the private temporary directories - will remain on the host's /tmp/ and /var/tmp/ directories. - If disconnected, the directories will be backed by a completely new tmpfs instance, - meaning that the storage is fully disconnected from the host namespace. Defaults to false. + the namespace, plus all temporary files created by a service in these directories will be removed + after the service is stopped. For this setting, the same restrictions regarding mount propagation + and privileges apply as for ReadOnlyPaths= and related calls, see above. This + setting is useful to secure access to temporary files of the process, but makes sharing between + processes via /tmp/ or /var/tmp/ impossible. If + DynamicUser= is enabled, disconnected is implied. Otherwise, + defaults to false. - This setting is useful to secure access to temporary files of the process, but makes sharing - between processes via /tmp/ or /var/tmp/ impossible. - If not set to disconnected, it is possible to run two or more units within - the same private /tmp/ and /var/tmp/ namespace by using - the JoinsNamespaceOf= directive, see - systemd.unit5 - for details. This setting is implied if DynamicUser= is set. For this setting, - the same restrictions regarding mount propagation and privileges apply as for - ReadOnlyPaths= and related calls, see above. If set to true - (as opposed to disconnected), this has the side effect of adding - Requires= and After= dependencies on all mount units necessary - to access /tmp/ and /var/tmp/ on the host. Moreover, an - implicitly After= ordering on + If true, the backing storage of the private temporary directories will + remain on the host's /tmp/ and /var/tmp/ directories. It is + possible to run two or more units within the same private /tmp/ and + /var/tmp/ namespace by using the JoinsNamespaceOf= directive, + see systemd.unit5 + for details. This has the side effect of adding Wants= and + After= dependencies on all mount units necessary to access + /tmp/ and /var/tmp/ on the host. Moreover, an implicit + After= ordering on systemd-tmpfiles-setup.service8 is added. + If disconnected, the directories will be backed by a completely new tmpfs + instance, meaning that the storage is fully disconnected from the host namespace. The tmpfs instance + does not shared with other units even if the JoinsNamespaceOf= directive is used. + If DefaultDependencies=no is specified, + RequiresMountsFor=/WantsMountsFor= for /var/ is not + specified, and RootDirectory=/RootImage= is not specified, then a new tmpfs is + mounted only on /tmp/, hence the host's + /var/tmp is still accessible from the unit. In that case, + $TMPDIR environment variable is set to /tmp to suggest the + processes in the unit to use /tmp/. This automatically adds + WantsMountsFor=/var/ dependency, unless DefaultDependencies=no + and/or RootDirectory=/RootImage= are specified. + + + Summary for <varname>PrivateTmp=disconnected</varname> + + + + + + + + Other Settings + tmpfs on /var/tmp/ + $TMPVAR + implied dependencies + + + + + (none) + yes + (unset) + WantsMountsFor=/var/ + + + RootDirectory=/RootImage= + yes + (unset) + (none) + + + DefaultDependency=no, RequiresMountsFor=/var/ + yes + (unset) + (none) + + + DefaultDependency=no, WantsMountsFor=/var/ + yes + (unset) + (none) + + + DefaultDependency=no + no + $TMPDIR=/tmp + (none) + + + +
+ Note that the implementation of this setting might be impossible (for example if mount namespaces are not available), and the unit should be written in a way that does not solely rely on this setting for security. @@ -3944,6 +4010,18 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX + + $TMPDIR + + The environment variable is set to /tmp when + PrivateTmp=disconnected specified with DefaultDependencies=no + and without RootDirectory=/RootImage= and + RequiresMountsFor=/WantsMountsFor= for /var/. + See explanation for PrivateTmp= above. + + + + $MAINPID