From e21fea24ae2a7a04f6d5c9d2bbbaf5833d248952 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Fri, 26 Jul 2013 05:22:22 +0200 Subject: [PATCH 1/1] rework systemd's own process environment handling/passing Stop importing non-sensical kernel-exported variables. All parameters in the kernel command line are exported to the initial environment of PID1, but suppressed if they are recognized by kernel built-in code. The EFI booted kernel will add further kernel-internal things which do not belong into userspace. The passed original environ data of the process is not touched and preserved across re-execution, to allow external reading of /proc/self/environ for process properties like container*=. --- man/systemd.xml | 15 +++------ src/core/locale-setup.c | 21 +++++++----- src/core/locale-setup.h | 2 +- src/core/main.c | 71 +++++++---------------------------------- src/core/manager.c | 47 +++++++++++++++++---------- src/core/manager.h | 2 +- src/shared/strv.c | 15 +++++++++ src/shared/strv.h | 1 + 8 files changed, 77 insertions(+), 97 deletions(-) diff --git a/man/systemd.xml b/man/systemd.xml index 1e54eac92..32bca0b60 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -1113,16 +1113,11 @@ systemd.setenv= Takes a string - argument in the form - VARIABLE=VALUE. May be used to set - environment variables for the init - process and all its children at boot - time. May be used more than once to - set multiple variables. If the equal - sign and variable are missing it unsets - an environment variable which might be - passed in from the initial ram - disk. + argument in the form VARIABLE=VALUE. + May be used to set default environment + variables to add to forked child processes. + May be used more than once to set multiple + variables. diff --git a/src/core/locale-setup.c b/src/core/locale-setup.c index daf81d080..31374ac64 100644 --- a/src/core/locale-setup.c +++ b/src/core/locale-setup.c @@ -28,6 +28,7 @@ #include "macro.h" #include "virt.h" #include "fileio.h" +#include "strv.h" enum { /* We don't list LC_ALL here on purpose. People should be @@ -67,7 +68,8 @@ static const char * const variable_names[_VARIABLE_MAX] = { [VARIABLE_LC_IDENTIFICATION] = "LC_IDENTIFICATION" }; -int locale_setup(void) { +int locale_setup(char ***environment) { + char **env; char *variables[_VARIABLE_MAX] = {}; int r = 0, i; @@ -118,13 +120,16 @@ int locale_setup(void) { } for (i = 0; i < _VARIABLE_MAX; i++) { - if (variables[i]) { - if (setenv(variable_names[i], variables[i], 1) < 0) { - r = -errno; - goto finish; - } - } else - unsetenv(variable_names[i]); + if (!variables[i]) + continue; + + env = strv_appendf(*environment, "%s=%s", variable_names[i], variables[i]); + if (!env) { + r = -ENOMEM; + goto finish; + } + + *environment = env; } r = 0; diff --git a/src/core/locale-setup.h b/src/core/locale-setup.h index 5a0f2f788..62c654c37 100644 --- a/src/core/locale-setup.h +++ b/src/core/locale-setup.h @@ -21,4 +21,4 @@ along with systemd; If not, see . ***/ -int locale_setup(void); +int locale_setup(char ***environment); diff --git a/src/core/main.c b/src/core/main.c index ad155e13c..91cbee2e5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -64,7 +64,6 @@ #endif #include "hostname-setup.h" #include "machine-id-setup.h" -#include "locale-setup.h" #include "selinux-setup.h" #include "ima-setup.h" #include "fileio.h" @@ -349,32 +348,21 @@ static int parse_proc_cmdline_word(const char *word) { arg_default_std_error = r; } else if (startswith(word, "systemd.setenv=")) { _cleanup_free_ char *cenv = NULL; - char *eq; - int r; cenv = strdup(word + 15); if (!cenv) return -ENOMEM; - eq = strchr(cenv, '='); - if (!eq) { - if (!env_name_is_valid(cenv)) - log_warning("Environment variable name '%s' is not valid. Ignoring.", cenv); - else { - r = unsetenv(cenv); - if (r < 0) - log_warning("Unsetting environment variable '%s' failed, ignoring: %m", cenv); - } - } else { - if (!env_assignment_is_valid(cenv)) - log_warning("Environment variable assignment '%s' is not valid. Ignoring.", cenv); - else { - *eq = 0; - r = setenv(cenv, eq + 1, 1); - if (r < 0) - log_warning("Setting environment variable '%s=%s' failed, ignoring: %m", cenv, eq + 1); - } - } + if (env_assignment_is_valid(cenv)) { + char **env; + + env = strv_env_set(arg_default_environment, cenv); + if (env) + arg_default_environment = env; + else + log_warning("Setting environment variable '%s' failed, ignoring: %m", cenv); + } else + log_warning("Environment variable name '%s' is not valid. Ignoring.", cenv); } else if (startswith(word, "systemd.") || (in_initrd() && startswith(word, "rd.systemd."))) { @@ -1445,42 +1433,7 @@ int main(int argc, char *argv[]) { if (serialization) assert_se(fdset_remove(fds, fileno(serialization)) >= 0); - /* Set up PATH unless it is already set */ - setenv("PATH", -#ifdef HAVE_SPLIT_USR - "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", -#else - "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin", -#endif - arg_running_as == SYSTEMD_SYSTEM); - if (arg_running_as == SYSTEMD_SYSTEM) { - /* Unset some environment variables passed in from the - * kernel that don't really make sense for us. */ - unsetenv("HOME"); - unsetenv("TERM"); - - /* When we are invoked by a shell, these might be set, - * but make little sense to pass on */ - unsetenv("PWD"); - unsetenv("SHLVL"); - unsetenv("_"); - - /* When we are invoked by a chroot-like tool such as - * nspawn, these might be set, but make little sense - * to pass on */ - unsetenv("USER"); - unsetenv("LOGNAME"); - - /* We suppress the socket activation env vars, as - * we'll try to match *any* open fd to units if - * possible. */ - unsetenv("LISTEN_FDS"); - unsetenv("LISTEN_PID"); - - /* All other variables are left as is, so that clients - * can still read them via /proc/1/environ */ - /* Become a session leader if we aren't one yet. */ setsid(); @@ -1528,8 +1481,6 @@ int main(int argc, char *argv[]) { log_debug(PACKAGE_STRING " running in user mode. (" SYSTEMD_FEATURES ")"); if (arg_running_as == SYSTEMD_SYSTEM && !skip_setup) { - locale_setup(); - if (arg_show_status || plymouth_running()) status_welcome(); @@ -1595,7 +1546,7 @@ int main(int argc, char *argv[]) { manager_set_default_rlimits(m, arg_default_rlimit); if (arg_default_environment) - manager_set_default_environment(m, arg_default_environment); + manager_environment_add(m, arg_default_environment); manager_set_show_status(m, arg_show_status); diff --git a/src/core/manager.c b/src/core/manager.c index 6c082c96b..10ccffb40 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -55,6 +55,7 @@ #include "util.h" #include "mkdir.h" #include "ratelimit.h" +#include "locale-setup.h" #include "mount-setup.h" #include "unit-name.h" #include "dbus-unit.h" @@ -454,22 +455,36 @@ static int manager_setup_signals(Manager *m) { return 0; } -static void manager_strip_environment(Manager *m) { +static int manager_default_environment(Manager *m) { +#ifdef HAVE_SPLIT_USR + const char *path = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"; +#else + const char *path = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin"; +#endif + assert(m); - /* Remove variables from the inherited set that are part of - * the container interface: - * http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface */ - strv_remove_prefix(m->environment, "container="); - strv_remove_prefix(m->environment, "container_"); + if (m->running_as == SYSTEMD_SYSTEM) { + /* The system manager always starts with a clean + * environment for its children. It does not import + * the kernel or the parents exported variables. + * + * The initial passed environ is untouched to keep + * /proc/self/environ valid; it is used for tagging + * the init process inside containers. */ + m->environment = strv_new(path, NULL); + + /* Import locale variables LC_*= from configuration */ + locale_setup(&m->environment); + } else + /* The user manager passes its own environment + * along to its children. */ + m->environment = strv_copy(environ); - /* Remove variables from the inherited set that are part of - * the initrd interface: - * http://www.freedesktop.org/wiki/Software/systemd/InitrdInterface */ - strv_remove_prefix(m->environment, "RD_"); + if (!m->environment) + return -ENOMEM; - /* Drop invalid entries */ - strv_env_clean(m->environment); + return 0; } int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **_m) { @@ -505,12 +520,10 @@ int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **_m) { m->epoll_fd = m->dev_autofs_fd = -1; m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */ - m->environment = strv_copy(environ); - if (!m->environment) + r = manager_default_environment(m); + if (r < 0) goto fail; - manager_strip_environment(m); - if (!(m->units = hashmap_new(string_hash_func, string_compare_func))) goto fail; @@ -2651,7 +2664,7 @@ void manager_undo_generators(Manager *m) { remove_generator_dir(m, &m->generator_unit_path_late); } -int manager_set_default_environment(Manager *m, char **environment) { +int manager_environment_add(Manager *m, char **environment) { char **e = NULL; assert(m); diff --git a/src/core/manager.h b/src/core/manager.h index bd068ac3c..3969553e3 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -281,7 +281,7 @@ unsigned manager_dispatch_load_queue(Manager *m); unsigned manager_dispatch_run_queue(Manager *m); unsigned manager_dispatch_dbus_queue(Manager *m); -int manager_set_default_environment(Manager *m, char **environment); +int manager_environment_add(Manager *m, char **environment); int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit); int manager_loop(Manager *m); diff --git a/src/shared/strv.c b/src/shared/strv.c index a5ce7e959..3e7778d61 100644 --- a/src/shared/strv.c +++ b/src/shared/strv.c @@ -387,6 +387,21 @@ fail: return NULL; } +char **strv_appendf(char **l, const char *format, ...) { + va_list ap; + _cleanup_free_ char *s = NULL; + int r; + + va_start(ap, format); + r = vasprintf(&s, format, ap); + va_end(ap); + + if (r < 0) + return NULL; + + return strv_append(l, s); +} + int strv_push(char ***l, char *value) { char **c; unsigned n; diff --git a/src/shared/strv.h b/src/shared/strv.h index e35118752..4ade827a4 100644 --- a/src/shared/strv.h +++ b/src/shared/strv.h @@ -42,6 +42,7 @@ unsigned strv_length(char * const *l) _pure_; char **strv_merge(char **a, char **b); char **strv_merge_concat(char **a, char **b, const char *suffix); char **strv_append(char **l, const char *s); +char **strv_appendf(char **l, const char *format, ...) _printf_attr_(2, 3); int strv_extend(char ***l, const char *value); int strv_push(char ***l, char *value); -- 2.30.2