From e8a565cb660a7a11f76180fe441ba8e4f9383771 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 6 Feb 2018 16:00:34 +0900 Subject: [PATCH 1/2] core: make ExecRuntime be manager managed object Before this, each ExecRuntime object is owned by a unit. However, it may be shared with other units which enable JoinsNamespaceOf=. Thus, by the serialization/deserialization process, its sharing information, more specifically, reference counter is lost, and causes issue #7790. This makes ExecRuntime objects be managed by manager, and changes the serialization/deserialization process. Fixes #7790. --- src/core/execute.c | 575 ++++++++++++++++++++++++++++++--------------- src/core/execute.h | 19 +- src/core/manager.c | 14 ++ src/core/manager.h | 3 + src/core/mount.c | 9 +- src/core/service.c | 9 +- src/core/socket.c | 9 +- src/core/swap.c | 9 +- src/core/unit.c | 45 +--- 9 files changed, 454 insertions(+), 238 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 0df3971df6..30a608e689 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -82,6 +82,7 @@ #include "label.h" #include "log.h" #include "macro.h" +#include "manager.h" #include "missing.h" #include "mkdir.h" #include "namespace.h" @@ -4523,182 +4524,6 @@ int exec_command_append(ExecCommand *c, const char *path, ...) { return 0; } - -static int exec_runtime_allocate(ExecRuntime **rt) { - - if (*rt) - return 0; - - *rt = new0(ExecRuntime, 1); - if (!*rt) - return -ENOMEM; - - (*rt)->n_ref = 1; - (*rt)->netns_storage_socket[0] = (*rt)->netns_storage_socket[1] = -1; - - return 0; -} - -int exec_runtime_make(ExecRuntime **rt, ExecContext *c, const char *id) { - int r; - - assert(rt); - assert(c); - assert(id); - - if (*rt) - return 1; - - if (!c->private_network && !c->private_tmp) - return 0; - - r = exec_runtime_allocate(rt); - if (r < 0) - return r; - - if (c->private_network && (*rt)->netns_storage_socket[0] < 0) { - if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, (*rt)->netns_storage_socket) < 0) - return -errno; - } - - if (c->private_tmp && !(*rt)->tmp_dir) { - r = setup_tmp_dirs(id, &(*rt)->tmp_dir, &(*rt)->var_tmp_dir); - if (r < 0) - return r; - } - - return 1; -} - -ExecRuntime *exec_runtime_ref(ExecRuntime *r) { - assert(r); - assert(r->n_ref > 0); - - r->n_ref++; - return r; -} - -ExecRuntime *exec_runtime_unref(ExecRuntime *r) { - - if (!r) - return NULL; - - assert(r->n_ref > 0); - - r->n_ref--; - if (r->n_ref > 0) - return NULL; - - free(r->tmp_dir); - free(r->var_tmp_dir); - safe_close_pair(r->netns_storage_socket); - return mfree(r); -} - -int exec_runtime_serialize(Unit *u, ExecRuntime *rt, FILE *f, FDSet *fds) { - assert(u); - assert(f); - assert(fds); - - if (!rt) - return 0; - - if (rt->tmp_dir) - unit_serialize_item(u, f, "tmp-dir", rt->tmp_dir); - - if (rt->var_tmp_dir) - unit_serialize_item(u, f, "var-tmp-dir", rt->var_tmp_dir); - - if (rt->netns_storage_socket[0] >= 0) { - int copy; - - copy = fdset_put_dup(fds, rt->netns_storage_socket[0]); - if (copy < 0) - return copy; - - unit_serialize_item_format(u, f, "netns-socket-0", "%i", copy); - } - - if (rt->netns_storage_socket[1] >= 0) { - int copy; - - copy = fdset_put_dup(fds, rt->netns_storage_socket[1]); - if (copy < 0) - return copy; - - unit_serialize_item_format(u, f, "netns-socket-1", "%i", copy); - } - - return 0; -} - -int exec_runtime_deserialize_item(Unit *u, ExecRuntime **rt, const char *key, const char *value, FDSet *fds) { - int r; - - assert(rt); - assert(key); - assert(value); - - if (streq(key, "tmp-dir")) { - char *copy; - - r = exec_runtime_allocate(rt); - if (r < 0) - return log_oom(); - - copy = strdup(value); - if (!copy) - return log_oom(); - - free((*rt)->tmp_dir); - (*rt)->tmp_dir = copy; - - } else if (streq(key, "var-tmp-dir")) { - char *copy; - - r = exec_runtime_allocate(rt); - if (r < 0) - return log_oom(); - - copy = strdup(value); - if (!copy) - return log_oom(); - - free((*rt)->var_tmp_dir); - (*rt)->var_tmp_dir = copy; - - } else if (streq(key, "netns-socket-0")) { - int fd; - - r = exec_runtime_allocate(rt); - if (r < 0) - return log_oom(); - - if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) - log_unit_debug(u, "Failed to parse netns socket value: %s", value); - else { - safe_close((*rt)->netns_storage_socket[0]); - (*rt)->netns_storage_socket[0] = fdset_remove(fds, fd); - } - } else if (streq(key, "netns-socket-1")) { - int fd; - - r = exec_runtime_allocate(rt); - if (r < 0) - return log_oom(); - - if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) - log_unit_debug(u, "Failed to parse netns socket value: %s", value); - else { - safe_close((*rt)->netns_storage_socket[1]); - (*rt)->netns_storage_socket[1] = fdset_remove(fds, fd); - } - } else - return 0; - - return 1; -} - static void *remove_tmpdir_thread(void *p) { _cleanup_free_ char *path = p; @@ -4706,17 +4531,17 @@ static void *remove_tmpdir_thread(void *p) { return NULL; } -void exec_runtime_destroy(ExecRuntime *rt) { +static ExecRuntime* exec_runtime_free(ExecRuntime *rt, bool destroy) { int r; if (!rt) - return; + return NULL; - /* If there are multiple users of this, let's leave the stuff around */ - if (rt->n_ref > 1) - return; + if (rt->manager) + (void) hashmap_remove(rt->manager->exec_runtime_by_id, rt->id); - if (rt->tmp_dir) { + /* When destroy is true, then rm_rf tmp_dir and var_tmp_dir. */ + if (destroy && rt->tmp_dir) { log_debug("Spawning thread to nuke %s", rt->tmp_dir); r = asynchronous_job(remove_tmpdir_thread, rt->tmp_dir); @@ -4728,7 +4553,7 @@ void exec_runtime_destroy(ExecRuntime *rt) { rt->tmp_dir = NULL; } - if (rt->var_tmp_dir) { + if (destroy && rt->var_tmp_dir) { log_debug("Spawning thread to nuke %s", rt->var_tmp_dir); r = asynchronous_job(remove_tmpdir_thread, rt->var_tmp_dir); @@ -4740,7 +4565,391 @@ void exec_runtime_destroy(ExecRuntime *rt) { rt->var_tmp_dir = NULL; } + rt->id = mfree(rt->id); + rt->tmp_dir = mfree(rt->tmp_dir); + rt->var_tmp_dir = mfree(rt->var_tmp_dir); safe_close_pair(rt->netns_storage_socket); + return mfree(rt); +} + +static void exec_runtime_freep(ExecRuntime **rt) { + if (*rt) + (void) exec_runtime_free(*rt, false); +} + +static int exec_runtime_allocate(ExecRuntime **rt) { + assert(rt); + + *rt = new0(ExecRuntime, 1); + if (!*rt) + return -ENOMEM; + + (*rt)->netns_storage_socket[0] = (*rt)->netns_storage_socket[1] = -1; + return 0; +} + +static int exec_runtime_add( + Manager *m, + const char *id, + const char *tmp_dir, + const char *var_tmp_dir, + const int netns_storage_socket[2], + ExecRuntime **ret) { + + _cleanup_(exec_runtime_freep) ExecRuntime *rt = NULL; + int r; + + assert(m); + assert(id); + + r = hashmap_ensure_allocated(&m->exec_runtime_by_id, &string_hash_ops); + if (r < 0) + return r; + + r = exec_runtime_allocate(&rt); + if (r < 0) + return r; + + rt->id = strdup(id); + if (!rt->id) + return -ENOMEM; + + if (tmp_dir) { + rt->tmp_dir = strdup(tmp_dir); + if (!rt->tmp_dir) + return -ENOMEM; + + /* When tmp_dir is set, then we require var_tmp_dir is also set. */ + assert(var_tmp_dir); + rt->var_tmp_dir = strdup(var_tmp_dir); + if (!rt->var_tmp_dir) + return -ENOMEM; + } + + if (netns_storage_socket) { + rt->netns_storage_socket[0] = netns_storage_socket[0]; + rt->netns_storage_socket[1] = netns_storage_socket[1]; + } + + r = hashmap_put(m->exec_runtime_by_id, rt->id, rt); + if (r < 0) + return r; + + rt->manager = m; + + if (ret) + *ret = rt; + + /* do not remove created ExecRuntime object when the operation succeeds. */ + rt = NULL; + return 0; +} + +static int exec_runtime_make(Manager *m, const ExecContext *c, const char *id, ExecRuntime **ret) { + _cleanup_free_ char *tmp_dir = NULL, *var_tmp_dir = NULL; + _cleanup_close_pair_ int netns_storage_socket[2] = {-1, -1}; + int r; + + assert(m); + assert(c); + assert(id); + + /* It is not necessary to create ExecRuntime object. */ + if (!c->private_network && !c->private_tmp) + return 0; + + if (c->private_tmp) { + r = setup_tmp_dirs(id, &tmp_dir, &var_tmp_dir); + if (r < 0) + return r; + } + + if (c->private_network) { + if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, netns_storage_socket) < 0) + return -errno; + } + + r = exec_runtime_add(m, id, tmp_dir, var_tmp_dir, netns_storage_socket, ret); + if (r < 0) + return r; + + /* Avoid cleanup */ + netns_storage_socket[0] = -1; + netns_storage_socket[1] = -1; + return 1; +} + +int exec_runtime_acquire(Manager *m, const ExecContext *c, const char *id, bool create, ExecRuntime **ret) { + ExecRuntime *rt; + int r; + + assert(m); + assert(id); + assert(ret); + + rt = hashmap_get(m->exec_runtime_by_id, id); + if (rt) + /* We already have a ExecRuntime object, let's increase the ref count and reuse it */ + goto ref; + + if (!create) + return 0; + + /* If not found, then create a new object. */ + r = exec_runtime_make(m, c, id, &rt); + if (r <= 0) + /* When r == 0, it is not necessary to create ExecRuntime object. */ + return r; + +ref: + /* increment reference counter. */ + rt->n_ref++; + *ret = rt; + return 1; +} + +ExecRuntime *exec_runtime_unref(ExecRuntime *rt, bool destroy) { + if (!rt) + return NULL; + + assert(rt->n_ref > 0); + + rt->n_ref--; + if (rt->n_ref > 0) + return NULL; + + return exec_runtime_free(rt, destroy); +} + +int exec_runtime_serialize(const Manager *m, FILE *f, FDSet *fds) { + ExecRuntime *rt; + Iterator i; + + assert(m); + assert(f); + assert(fds); + + HASHMAP_FOREACH(rt, m->exec_runtime_by_id, i) { + fprintf(f, "exec-runtime=%s", rt->id); + + if (rt->tmp_dir) + fprintf(f, " tmp-dir=%s", rt->tmp_dir); + + if (rt->var_tmp_dir) + fprintf(f, " var-tmp-dir=%s", rt->var_tmp_dir); + + if (rt->netns_storage_socket[0] >= 0) { + int copy; + + copy = fdset_put_dup(fds, rt->netns_storage_socket[0]); + if (copy < 0) + return copy; + + fprintf(f, " netns-socket-0=%i", copy); + } + + if (rt->netns_storage_socket[1] >= 0) { + int copy; + + copy = fdset_put_dup(fds, rt->netns_storage_socket[1]); + if (copy < 0) + return copy; + + fprintf(f, " netns-socket-1=%i", copy); + } + + fputc('\n', f); + } + + return 0; +} + +int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value, FDSet *fds) { + _cleanup_(exec_runtime_freep) ExecRuntime *rt_create = NULL; + ExecRuntime *rt; + int r; + + /* This is for the migration from old (v237 or earlier) deserialization text. + * Due to the bug #7790, this may not work with the units that use JoinsNamespaceOf=. + * Even if the ExecRuntime object originally created by the other unit, we cannot judge + * so or not from the serialized text, then we always creates a new object owned by this. */ + + assert(u); + assert(key); + assert(value); + + /* Manager manages ExecRuntime objects by the unit id. + * So, we omit the serialized text when the unit does not have id (yet?)... */ + if (isempty(u->id)) { + log_unit_debug(u, "Invocation ID not found. Dropping runtime parameter."); + return 0; + } + + r = hashmap_ensure_allocated(&u->manager->exec_runtime_by_id, &string_hash_ops); + if (r < 0) { + log_unit_debug_errno(u, r, "Failed to allocate storage for runtime parameter: %m"); + return 0; + } + + rt = hashmap_get(u->manager->exec_runtime_by_id, u->id); + if (!rt) { + r = exec_runtime_allocate(&rt_create); + if (r < 0) + return log_oom(); + + rt_create->id = strdup(u->id); + if (!rt_create->id) + return log_oom(); + + rt = rt_create; + } + + if (streq(key, "tmp-dir")) { + char *copy; + + copy = strdup(value); + if (!copy) + return log_oom(); + + free_and_replace(rt->tmp_dir, copy); + + } else if (streq(key, "var-tmp-dir")) { + char *copy; + + copy = strdup(value); + if (!copy) + return log_oom(); + + free_and_replace(rt->var_tmp_dir, copy); + + } else if (streq(key, "netns-socket-0")) { + int fd; + + if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) { + log_unit_debug(u, "Failed to parse netns socket value: %s", value); + return 0; + } + + safe_close(rt->netns_storage_socket[0]); + rt->netns_storage_socket[0] = fdset_remove(fds, fd); + + } else if (streq(key, "netns-socket-1")) { + int fd; + + if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) { + log_unit_debug(u, "Failed to parse netns socket value: %s", value); + return 0; + } + + safe_close(rt->netns_storage_socket[1]); + rt->netns_storage_socket[1] = fdset_remove(fds, fd); + } else + return 0; + + + /* If the object is newly created, then put it to the hashmap which manages ExecRuntime objects. */ + if (rt_create) { + r = hashmap_put(u->manager->exec_runtime_by_id, rt_create->id, rt_create); + if (r < 0) { + log_unit_debug_errno(u, r, "Failed to put runtime paramter to manager's storage: %m"); + return 0; + } + + rt_create->manager = u->manager; + + /* Avoid cleanup */ + rt_create = NULL; + } + + return 1; +} + +void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) { + char *id = NULL, *tmp_dir = NULL, *var_tmp_dir = NULL; + int r, fd0 = -1, fd1 = -1; + const char *p, *v = value; + size_t n; + + assert(m); + assert(value); + assert(fds); + + n = strcspn(v, " "); + id = strndupa(v, n); + if (v[n] != ' ') + goto finalize; + p = v + n + 1; + + v = startswith(p, "tmp-dir="); + if (v) { + n = strcspn(v, " "); + tmp_dir = strndupa(v, n); + if (v[n] != ' ') + goto finalize; + p = v + n + 1; + } + + v = startswith(p, "var-tmp-dir="); + if (v) { + n = strcspn(v, " "); + var_tmp_dir = strndupa(v, n); + if (v[n] != ' ') + goto finalize; + p = v + n + 1; + } + + v = startswith(p, "netns-socket-0="); + if (v) { + char *buf; + + n = strcspn(v, " "); + buf = strndupa(v, n); + if (safe_atoi(buf, &fd0) < 0 || !fdset_contains(fds, fd0)) { + log_debug("Unable to process exec-runtime netns fd specification."); + return; + } + fd0 = fdset_remove(fds, fd0); + if (v[n] != ' ') + goto finalize; + p = v + n + 1; + } + + v = startswith(p, "netns-socket-1="); + if (v) { + char *buf; + + n = strcspn(v, " "); + buf = strndupa(v, n); + if (safe_atoi(buf, &fd1) < 0 || !fdset_contains(fds, fd1)) { + log_debug("Unable to process exec-runtime netns fd specification."); + return; + } + fd1 = fdset_remove(fds, fd1); + } + +finalize: + + r = exec_runtime_add(m, id, tmp_dir, var_tmp_dir, (int[]) { fd0, fd1 }, NULL); + if (r < 0) { + log_debug_errno(r, "Failed to add exec-runtime: %m"); + return; + } +} + +void exec_runtime_vacuum(Manager *m) { + ExecRuntime *rt; + Iterator i; + + assert(m); + + /* Free unreferenced ExecRuntime objects. This is used after manager deserialization process. */ + + HASHMAP_FOREACH(rt, m->exec_runtime_by_id, i) { + if (rt->n_ref > 0) + continue; + + (void) exec_runtime_free(rt, false); + } } static const char* const exec_input_table[_EXEC_INPUT_MAX] = { diff --git a/src/core/execute.h b/src/core/execute.h index d04dfcf6c9..90d0fc64e3 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -25,6 +25,7 @@ typedef struct ExecCommand ExecCommand; typedef struct ExecContext ExecContext; typedef struct ExecRuntime ExecRuntime; typedef struct ExecParameters ExecParameters; +typedef struct Manager Manager; #include #include @@ -120,6 +121,11 @@ struct ExecCommand { struct ExecRuntime { int n_ref; + Manager *manager; + + /* unit id of the owner */ + char *id; + char *tmp_dir; char *var_tmp_dir; @@ -374,14 +380,13 @@ void exec_status_start(ExecStatus *s, pid_t pid); void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, int status); void exec_status_dump(ExecStatus *s, FILE *f, const char *prefix); -int exec_runtime_make(ExecRuntime **rt, ExecContext *c, const char *id); -ExecRuntime *exec_runtime_ref(ExecRuntime *r); -ExecRuntime *exec_runtime_unref(ExecRuntime *r); +int exec_runtime_acquire(Manager *m, const ExecContext *c, const char *name, bool create, ExecRuntime **ret); +ExecRuntime *exec_runtime_unref(ExecRuntime *r, bool destroy); -int exec_runtime_serialize(Unit *unit, ExecRuntime *rt, FILE *f, FDSet *fds); -int exec_runtime_deserialize_item(Unit *unit, ExecRuntime **rt, const char *key, const char *value, FDSet *fds); - -void exec_runtime_destroy(ExecRuntime *rt); +int exec_runtime_serialize(const Manager *m, FILE *f, FDSet *fds); +int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value, FDSet *fds); +void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds); +void exec_runtime_vacuum(Manager *m); const char* exec_output_to_string(ExecOutput i) _const_; ExecOutput exec_output_from_string(const char *s) _pure_; diff --git a/src/core/manager.c b/src/core/manager.c index e837a46f56..5021e00b87 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1200,6 +1200,9 @@ Manager* manager_free(Manager *m) { bus_done(m); + exec_runtime_vacuum(m); + hashmap_free(m->exec_runtime_by_id); + dynamic_user_vacuum(m, false); hashmap_free(m->dynamic_users); @@ -1463,6 +1466,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { /* Release any dynamic users no longer referenced */ dynamic_user_vacuum(m, true); + exec_runtime_vacuum(m); + /* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */ manager_vacuum_uid_refs(m); manager_vacuum_gid_refs(m); @@ -2800,6 +2805,10 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { manager_serialize_uid_refs(m, f); manager_serialize_gid_refs(m, f); + r = exec_runtime_serialize(m, f, fds); + if (r < 0) + return r; + (void) fputc('\n', f); HASHMAP_FOREACH_KEY(u, t, m->units, i) { @@ -2978,6 +2987,8 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { manager_deserialize_uid_refs_one(m, val); else if ((val = startswith(l, "destroy-ipc-gid="))) manager_deserialize_gid_refs_one(m, val); + else if ((val = startswith(l, "exec-runtime="))) + exec_runtime_deserialize_one(m, val, fds); else if ((val = startswith(l, "subscribed="))) { if (strv_extend(&m->deserialized_subscribed, val) < 0) @@ -3082,6 +3093,7 @@ int manager_reload(Manager *m) { manager_clear_jobs_and_units(m); lookup_paths_flush_generator(&m->lookup_paths); lookup_paths_free(&m->lookup_paths); + exec_runtime_vacuum(m); dynamic_user_vacuum(m, false); m->uid_refs = hashmap_free(m->uid_refs); m->gid_refs = hashmap_free(m->gid_refs); @@ -3140,6 +3152,8 @@ int manager_reload(Manager *m) { manager_vacuum_uid_refs(m); manager_vacuum_gid_refs(m); + exec_runtime_vacuum(m); + /* It might be safe to log to the journal now. */ manager_recheck_journal(m); diff --git a/src/core/manager.h b/src/core/manager.h index 0eed67b46a..c785861bfb 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -341,6 +341,9 @@ struct Manager { Hashmap *uid_refs; Hashmap *gid_refs; + /* ExecRuntime, indexed by their owner unit id */ + Hashmap *exec_runtime_by_id; + /* When the user hits C-A-D more than 7 times per 2s, do something immediately... */ RateLimit ctrl_alt_del_ratelimit; EmergencyAction cad_burst_action; diff --git a/src/core/mount.c b/src/core/mount.c index e72d8161ed..76883475a2 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -241,7 +241,7 @@ static void mount_done(Unit *u) { mount_parameters_done(&m->parameters_proc_self_mountinfo); mount_parameters_done(&m->parameters_fragment); - m->exec_runtime = exec_runtime_unref(m->exec_runtime); + m->exec_runtime = exec_runtime_unref(m->exec_runtime, false); exec_command_done_array(m->exec_command, _MOUNT_EXEC_COMMAND_MAX); m->control_command = NULL; @@ -699,8 +699,10 @@ static int mount_coldplug(Unit *u) { return r; } - if (!IN_SET(new_state, MOUNT_DEAD, MOUNT_FAILED)) + if (!IN_SET(new_state, MOUNT_DEAD, MOUNT_FAILED)) { (void) unit_setup_dynamic_creds(u); + (void) unit_setup_exec_runtime(u); + } mount_set_state(m, new_state); return 0; @@ -813,8 +815,7 @@ static void mount_enter_dead(Mount *m, MountResult f) { mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD); - exec_runtime_destroy(m->exec_runtime); - m->exec_runtime = exec_runtime_unref(m->exec_runtime); + m->exec_runtime = exec_runtime_unref(m->exec_runtime, true); exec_context_destroy_runtime_directory(&m->exec_context, UNIT(m)->manager->prefix[EXEC_DIRECTORY_RUNTIME]); diff --git a/src/core/service.c b/src/core/service.c index 0b53b1c829..ff0a82864f 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -369,7 +369,7 @@ static void service_done(Unit *u) { s->pid_file = mfree(s->pid_file); s->status_text = mfree(s->status_text); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, false); exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX); s->control_command = NULL; s->main_command = NULL; @@ -1155,8 +1155,10 @@ static int service_coldplug(Unit *u) { if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) service_start_watchdog(s); - if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) + if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) { (void) unit_setup_dynamic_creds(u); + (void) unit_setup_exec_runtime(u); + } if (UNIT_ISSET(s->accept_socket)) { Socket* socket = SOCKET(UNIT_DEREF(s->accept_socket)); @@ -1643,8 +1645,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) s->forbid_restart = false; /* We want fresh tmpdirs in case service is started again immediately */ - exec_runtime_destroy(s->exec_runtime); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); if (s->exec_context.runtime_directory_preserve_mode == EXEC_PRESERVE_NO || (s->exec_context.runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART && !service_will_restart(UNIT(s)))) diff --git a/src/core/socket.c b/src/core/socket.c index 74cdebbe81..703f9f760f 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -164,7 +164,7 @@ static void socket_done(Unit *u) { s->peers_by_address = set_free(s->peers_by_address); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, false); exec_command_free_array(s->exec_command, _SOCKET_EXEC_COMMAND_MAX); s->control_command = NULL; @@ -1878,8 +1878,10 @@ static int socket_coldplug(Unit *u) { return r; } - if (!IN_SET(s->deserialized_state, SOCKET_DEAD, SOCKET_FAILED)) + if (!IN_SET(s->deserialized_state, SOCKET_DEAD, SOCKET_FAILED)) { (void) unit_setup_dynamic_creds(u); + (void) unit_setup_exec_runtime(u); + } socket_set_state(s, s->deserialized_state); return 0; @@ -2017,8 +2019,7 @@ static void socket_enter_dead(Socket *s, SocketResult f) { socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD); - exec_runtime_destroy(s->exec_runtime); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); exec_context_destroy_runtime_directory(&s->exec_context, UNIT(s)->manager->prefix[EXEC_DIRECTORY_RUNTIME]); diff --git a/src/core/swap.c b/src/core/swap.c index b10066db01..3b234c2212 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -157,7 +157,7 @@ static void swap_done(Unit *u) { s->parameters_fragment.what = mfree(s->parameters_fragment.what); s->parameters_fragment.options = mfree(s->parameters_fragment.options); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, false); exec_command_done_array(s->exec_command, _SWAP_EXEC_COMMAND_MAX); s->control_command = NULL; @@ -549,8 +549,10 @@ static int swap_coldplug(Unit *u) { return r; } - if (!IN_SET(new_state, SWAP_DEAD, SWAP_FAILED)) + if (!IN_SET(new_state, SWAP_DEAD, SWAP_FAILED)) { (void) unit_setup_dynamic_creds(u); + (void) unit_setup_exec_runtime(u); + } swap_set_state(s, new_state); return 0; @@ -669,8 +671,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) { swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD); - exec_runtime_destroy(s->exec_runtime); - s->exec_runtime = exec_runtime_unref(s->exec_runtime); + s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); exec_context_destroy_runtime_directory(&s->exec_context, UNIT(s)->manager->prefix[EXEC_DIRECTORY_RUNTIME]); diff --git a/src/core/unit.c b/src/core/unit.c index 54e298cae4..4a5fa4005c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3176,18 +3176,9 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs) { assert(fds); if (unit_can_serialize(u)) { - ExecRuntime *rt; - r = UNIT_VTABLE(u)->serialize(u, f, fds); if (r < 0) return r; - - rt = unit_get_exec_runtime(u); - if (rt) { - r = exec_runtime_serialize(u, rt, f, fds); - if (r < 0) - return r; - } } dual_timestamp_serialize(f, "state-change-timestamp", &u->state_change_timestamp); @@ -3333,18 +3324,12 @@ void unit_serialize_item_format(Unit *u, FILE *f, const char *key, const char *f } int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { - ExecRuntime **rt = NULL; - size_t offset; int r; assert(u); assert(f); assert(fds); - offset = UNIT_VTABLE(u)->exec_runtime_offset; - if (offset > 0) - rt = (ExecRuntime**) ((uint8_t*) u + offset); - for (;;) { char line[LINE_MAX], *l, *v; CGroupIPAccountingMetric m; @@ -3604,18 +3589,16 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { } if (unit_can_serialize(u)) { - if (rt) { - r = exec_runtime_deserialize_item(u, rt, l, v, fds); - if (r < 0) { - log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l); - continue; - } - - /* Returns positive if key was handled by the call */ - if (r > 0) - continue; + r = exec_runtime_deserialize_compat(u, l, v, fds); + if (r < 0) { + log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l); + continue; } + /* Returns positive if key was handled by the call */ + if (r > 0) + continue; + r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds); if (r < 0) log_unit_warning(u, "Failed to deserialize unit parameter '%s', ignoring.", l); @@ -4684,6 +4667,7 @@ int unit_setup_exec_runtime(Unit *u) { Unit *other; Iterator i; void *v; + int r; offset = UNIT_VTABLE(u)->exec_runtime_offset; assert(offset > 0); @@ -4695,15 +4679,12 @@ int unit_setup_exec_runtime(Unit *u) { /* Try to get it from somebody else */ HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_JOINS_NAMESPACE_OF], i) { - - *rt = unit_get_exec_runtime(other); - if (*rt) { - exec_runtime_ref(*rt); - return 0; - } + r = exec_runtime_acquire(u->manager, NULL, other->id, false, rt); + if (r == 1) + return 1; } - return exec_runtime_make(rt, unit_get_exec_context(u), u->id); + return exec_runtime_acquire(u->manager, unit_get_exec_context(u), u->id, true, rt); } int unit_setup_dynamic_creds(Unit *u) { From 34cf6c43406d580648ee79503cba560b25940bf4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 6 Feb 2018 12:17:50 +0900 Subject: [PATCH 2/2] core/execute: make arguments constant if possible Also make functions static if possible. --- src/basic/time-util.h | 4 +-- src/core/execute.c | 59 +++++++++++++++++++++++-------------------- src/core/execute.h | 20 +++++---------- 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/basic/time-util.h b/src/basic/time-util.h index dc4a159310..a8c5a8375b 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -100,12 +100,12 @@ triple_timestamp* triple_timestamp_from_realtime(triple_timestamp *ts, usec_t u) #define TRIPLE_TIMESTAMP_HAS_CLOCK(clock) \ IN_SET(clock, CLOCK_REALTIME, CLOCK_REALTIME_ALARM, CLOCK_MONOTONIC, CLOCK_BOOTTIME, CLOCK_BOOTTIME_ALARM) -static inline bool dual_timestamp_is_set(dual_timestamp *ts) { +static inline bool dual_timestamp_is_set(const dual_timestamp *ts) { return ((ts->realtime > 0 && ts->realtime != USEC_INFINITY) || (ts->monotonic > 0 && ts->monotonic != USEC_INFINITY)); } -static inline bool triple_timestamp_is_set(triple_timestamp *ts) { +static inline bool triple_timestamp_is_set(const triple_timestamp *ts) { return ((ts->realtime > 0 && ts->realtime != USEC_INFINITY) || (ts->monotonic > 0 && ts->monotonic != USEC_INFINITY) || (ts->boottime > 0 && ts->boottime != USEC_INFINITY)); diff --git a/src/core/execute.c b/src/core/execute.c index 30a608e689..be9ef20725 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -331,7 +331,7 @@ static int connect_journal_socket(int fd, uid_t uid, gid_t gid) { } static int connect_logger_as( - Unit *unit, + const Unit *unit, const ExecContext *context, const ExecParameters *params, ExecOutput output, @@ -562,7 +562,7 @@ static int setup_input( } static int setup_output( - Unit *unit, + const Unit *unit, const ExecContext *context, const ExecParameters *params, int fileno, @@ -1055,7 +1055,7 @@ static int get_supplementary_groups(const ExecContext *c, const char *user, return 0; } -static int enforce_groups(gid_t gid, gid_t *supplementary_gids, int ngids) { +static int enforce_groups(gid_t gid, const gid_t *supplementary_gids, int ngids) { int r; /* Handle SupplementaryGroups= if it is not empty */ @@ -1541,7 +1541,7 @@ static int apply_private_devices(const Unit *u, const ExecContext *c) { return seccomp_load_syscall_filter_set(SCMP_ACT_ALLOW, syscall_filter_sets + SYSCALL_FILTER_SET_RAW_IO, SCMP_ACT_ERRNO(EPERM)); } -static int apply_restrict_namespaces(Unit *u, const ExecContext *c) { +static int apply_restrict_namespaces(const Unit *u, const ExecContext *c) { assert(u); assert(c); @@ -1611,7 +1611,7 @@ static void do_idle_pipe_dance(int idle_pipe[4]) { } static int build_environment( - Unit *u, + const Unit *u, const ExecContext *c, const ExecParameters *p, unsigned n_fds, @@ -2310,11 +2310,11 @@ finish: } static int apply_mount_namespace( - Unit *u, - ExecCommand *command, + const Unit *u, + const ExecCommand *command, const ExecContext *context, const ExecParameters *params, - ExecRuntime *runtime) { + const ExecRuntime *runtime) { _cleanup_strv_free_ char **empty_directories = NULL; char *tmp = NULL, *var = NULL; @@ -2438,7 +2438,7 @@ static int apply_working_directory( } static int setup_keyring( - Unit *u, + const Unit *u, const ExecContext *context, const ExecParameters *p, uid_t uid, gid_t gid) { @@ -2549,7 +2549,7 @@ static int setup_keyring( return 0; } -static void append_socket_pair(int *array, unsigned *n, int pair[2]) { +static void append_socket_pair(int *array, unsigned *n, const int pair[2]) { assert(array); assert(n); @@ -2564,8 +2564,8 @@ static void append_socket_pair(int *array, unsigned *n, int pair[2]) { static int close_remaining_fds( const ExecParameters *params, - ExecRuntime *runtime, - DynamicCreds *dcreds, + const ExecRuntime *runtime, + const DynamicCreds *dcreds, int user_lookup_fd, int socket_fd, int *fds, unsigned n_fds) { @@ -2708,9 +2708,11 @@ static int compile_suggested_paths(const ExecContext *c, const ExecParameters *p return 0; } +static char *exec_command_line(char **argv); + static int exec_child( Unit *unit, - ExecCommand *command, + const ExecCommand *command, const ExecContext *context, const ExecParameters *params, ExecRuntime *runtime, @@ -3442,6 +3444,9 @@ static int exec_child( return log_unit_error_errno(unit, errno, "Failed to execute command: %m"); } +static int exec_context_load_environment(const Unit *unit, const ExecContext *c, char ***l); +static int exec_context_named_iofds(const ExecContext *c, const ExecParameters *p, int named_iofds[3]); + int exec_spawn(Unit *unit, ExecCommand *command, const ExecContext *context, @@ -3488,7 +3493,7 @@ int exec_spawn(Unit *unit, n_socket_fds = params->n_socket_fds; } - r = exec_context_named_iofds(unit, context, params, named_iofds); + r = exec_context_named_iofds(context, params, named_iofds); if (r < 0) return log_unit_error_errno(unit, r, "Failed to load a named file descriptor: %m"); @@ -3641,7 +3646,7 @@ void exec_context_done(ExecContext *c) { c->stdin_data_size = 0; } -int exec_context_destroy_runtime_directory(ExecContext *c, const char *runtime_prefix) { +int exec_context_destroy_runtime_directory(const ExecContext *c, const char *runtime_prefix) { char **i; assert(c); @@ -3664,7 +3669,7 @@ int exec_context_destroy_runtime_directory(ExecContext *c, const char *runtime_p return 0; } -void exec_command_done(ExecCommand *c) { +static void exec_command_done(ExecCommand *c) { assert(c); c->path = mfree(c->path); @@ -3699,7 +3704,7 @@ void exec_command_free_array(ExecCommand **c, unsigned n) { } typedef struct InvalidEnvInfo { - Unit *unit; + const Unit *unit; const char *path; } InvalidEnvInfo; @@ -3737,7 +3742,7 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index) { } } -int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParameters *p, int named_iofds[3]) { +static int exec_context_named_iofds(const ExecContext *c, const ExecParameters *p, int named_iofds[3]) { unsigned i, targets; const char* stdio_fdname[3]; unsigned n_fds; @@ -3783,7 +3788,7 @@ int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParamet return targets == 0 ? 0 : -ENOENT; } -int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) { +static int exec_context_load_environment(const Unit *unit, const ExecContext *c, char ***l) { char **i, **r = NULL; assert(c); @@ -3887,7 +3892,7 @@ static bool tty_may_match_dev_console(const char *tty) { return streq(console, tty) || (streq(console, "tty0") && tty_is_vc(tty)); } -bool exec_context_may_touch_console(ExecContext *ec) { +bool exec_context_may_touch_console(const ExecContext *ec) { return (ec->tty_reset || ec->tty_vhangup || @@ -3907,7 +3912,7 @@ static void strv_fprintf(FILE *f, char **l) { fprintf(f, " %s", *g); } -void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) { +void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { ExecDirectoryType dt; char **e, **d; unsigned i; @@ -4293,7 +4298,7 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) { prefix, c->apparmor_profile_ignore ? "-" : "", c->apparmor_profile); } -bool exec_context_maintains_privileges(ExecContext *c) { +bool exec_context_maintains_privileges(const ExecContext *c) { assert(c); /* Returns true if the process forked off would run under @@ -4308,7 +4313,7 @@ bool exec_context_maintains_privileges(ExecContext *c) { return false; } -int exec_context_get_effective_ioprio(ExecContext *c) { +int exec_context_get_effective_ioprio(const ExecContext *c) { int p; assert(c); @@ -4342,7 +4347,7 @@ void exec_status_start(ExecStatus *s, pid_t pid) { dual_timestamp_get(&s->start_timestamp); } -void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, int status) { +void exec_status_exit(ExecStatus *s, const ExecContext *context, pid_t pid, int code, int status) { assert(s); if (s->pid && s->pid != pid) @@ -4362,7 +4367,7 @@ void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, } } -void exec_status_dump(ExecStatus *s, FILE *f, const char *prefix) { +void exec_status_dump(const ExecStatus *s, FILE *f, const char *prefix) { char buf[FORMAT_TIMESTAMP_MAX]; assert(s); @@ -4392,7 +4397,7 @@ void exec_status_dump(ExecStatus *s, FILE *f, const char *prefix) { prefix, s->status); } -char *exec_command_line(char **argv) { +static char *exec_command_line(char **argv) { size_t k; char *n, *p, **a; bool first = true; @@ -4432,7 +4437,7 @@ char *exec_command_line(char **argv) { return n; } -void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix) { +static void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix) { _cleanup_free_ char *cmd = NULL; const char *prefix2; diff --git a/src/core/execute.h b/src/core/execute.h index 90d0fc64e3..4023b30164 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -345,15 +345,11 @@ int exec_spawn(Unit *unit, DynamicCreds *dynamic_creds, pid_t *ret); -void exec_command_done(ExecCommand *c); void exec_command_done_array(ExecCommand *c, unsigned n); ExecCommand* exec_command_free_list(ExecCommand *c); void exec_command_free_array(ExecCommand **c, unsigned n); -char *exec_command_line(char **argv); - -void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix); void exec_command_dump_list(ExecCommand *c, FILE *f, const char *prefix); void exec_command_append_list(ExecCommand **l, ExecCommand *e); int exec_command_set(ExecCommand *c, const char *path, ...); @@ -361,24 +357,22 @@ int exec_command_append(ExecCommand *c, const char *path, ...); void exec_context_init(ExecContext *c); void exec_context_done(ExecContext *c); -void exec_context_dump(ExecContext *c, FILE* f, const char *prefix); +void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix); -int exec_context_destroy_runtime_directory(ExecContext *c, const char *runtime_root); +int exec_context_destroy_runtime_directory(const ExecContext *c, const char *runtime_root); -int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l); -int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParameters *p, int named_iofds[3]); const char* exec_context_fdname(const ExecContext *c, int fd_index); -bool exec_context_may_touch_console(ExecContext *c); -bool exec_context_maintains_privileges(ExecContext *c); +bool exec_context_may_touch_console(const ExecContext *c); +bool exec_context_maintains_privileges(const ExecContext *c); -int exec_context_get_effective_ioprio(ExecContext *c); +int exec_context_get_effective_ioprio(const ExecContext *c); void exec_context_free_log_extra_fields(ExecContext *c); void exec_status_start(ExecStatus *s, pid_t pid); -void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, int status); -void exec_status_dump(ExecStatus *s, FILE *f, const char *prefix); +void exec_status_exit(ExecStatus *s, const ExecContext *context, pid_t pid, int code, int status); +void exec_status_dump(const ExecStatus *s, FILE *f, const char *prefix); int exec_runtime_acquire(Manager *m, const ExecContext *c, const char *name, bool create, ExecRuntime **ret); ExecRuntime *exec_runtime_unref(ExecRuntime *r, bool destroy);