core: rework context initialization/destruction logic

Let's automatically initialize the kill, exec and cgroup contexts of the
various unit types when the object is constructed, instead of
invididually in type-specific code.

Also, when PrivateDevices= is set, set DevicePolicy= to closed.

src/core/mount.c

@@ -123,12 +123,6 @@ static void mount_init(Unit *u) {
         m->timeout_usec = u->manager->default_timeout_start_usec;
         m->directory_mode = 0755;
 
-        exec_context_init(&m->exec_context);
-        kill_context_init(&m->kill_context);
-        cgroup_context_init(&m->cgroup_context);
-
-        unit_cgroup_context_init_defaults(u, &m->cgroup_context);
-
         if (unit_has_name(u, "-.mount")) {
                 /* Don't allow start/stop for root directory */
                 u->refuse_manual_start = true;
@@ -203,8 +197,6 @@ static void mount_done(Unit *u) {
         mount_parameters_done(&m->parameters_proc_self_mountinfo);
         mount_parameters_done(&m->parameters_fragment);
 
-        cgroup_context_done(&m->cgroup_context);
-        exec_context_done(&m->exec_context);
         m->exec_runtime = exec_runtime_unref(m->exec_runtime);
         exec_command_done_array(m->exec_command, _MOUNT_EXEC_COMMAND_MAX);
         m->control_command = NULL;
@@ -529,10 +521,6 @@ static int mount_add_extras(Mount *m) {
 
         path_kill_slashes(m->where);
 
-        r = unit_add_exec_dependencies(u, &m->exec_context);
-        if (r < 0)
-                return r;
-
         if (!u->description) {
                 r = unit_set_description(u, m->where);
                 if (r < 0)
@@ -551,13 +539,15 @@ static int mount_add_extras(Mount *m) {
         if (r < 0)
                 return r;
 
-        if (u->default_dependencies) {
-                r = mount_add_default_dependencies(m);
-                if (r < 0)
-                        return r;
-        }
+        r = unit_patch_contexts(u);
+        if (r < 0)
+                return r;
 
-        r = unit_add_default_slice(u);
+        r = unit_add_exec_dependencies(u, &m->exec_context);
+        if (r < 0)
+                return r;
+
+        r = unit_add_default_slice(u, &m->cgroup_context);
         if (r < 0)
                 return r;
 
@@ -565,9 +555,11 @@ static int mount_add_extras(Mount *m) {
         if (r < 0)
                 return r;
 
-        r = unit_exec_context_patch_defaults(u, &m->exec_context);
-        if (r < 0)
-                return r;
+        if (u->default_dependencies) {
+                r = mount_add_default_dependencies(m);
+                if (r < 0)
+                        return r;
+        }
 
         return 0;
 }

src/core/scope.c

@@ -51,11 +51,6 @@ static void scope_init(Unit *u) {
 
         s->timeout_stop_usec = u->manager->default_timeout_stop_usec;
 
-        cgroup_context_init(&s->cgroup_context);
-        kill_context_init(&s->kill_context);
-
-        unit_cgroup_context_init_defaults(u, &s->cgroup_context);
-
         UNIT(s)->ignore_on_isolate = true;
         UNIT(s)->ignore_on_snapshot = true;
 }
@@ -65,8 +60,6 @@ static void scope_done(Unit *u) {
 
         assert(u);
 
-        cgroup_context_done(&s->cgroup_context);
-
         free(s->controller);
 
         s->timer_event_source = sd_event_source_unref(s->timer_event_source);
@@ -158,7 +151,11 @@ static int scope_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        r = unit_add_default_slice(u);
+        r = unit_patch_contexts(u);
+        if (r < 0)
+                return r;
+
+        r = unit_add_default_slice(u, &s->cgroup_context);
         if (r < 0)
                 return r;
 

src/core/service.c

@@ -143,12 +143,6 @@ static void service_init(Unit *u) {
         s->socket_fd = -1;
         s->guess_main_pid = true;
 
-        exec_context_init(&s->exec_context);
-        kill_context_init(&s->kill_context);
-        cgroup_context_init(&s->cgroup_context);
-
-        unit_cgroup_context_init_defaults(u, &s->cgroup_context);
-
         RATELIMIT_INIT(s->start_limit, u->manager->default_start_limit_interval, u->manager->default_start_limit_burst);
 
         s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID;
@@ -301,8 +295,6 @@ static void service_done(Unit *u) {
         free(s->status_text);
         s->status_text = NULL;
 
-        cgroup_context_done(&s->cgroup_context);
-        exec_context_done(&s->exec_context);
         s->exec_runtime = exec_runtime_unref(s->exec_runtime);
         exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX);
         s->control_command = NULL;
@@ -1202,11 +1194,15 @@ static int service_load(Unit *u) {
 
                 service_fix_output(s);
 
+                r = unit_patch_contexts(u);
+                if (r < 0)
+                        return r;
+
                 r = unit_add_exec_dependencies(u, &s->exec_context);
                 if (r < 0)
                         return r;
 
-                r = unit_add_default_slice(u);
+                r = unit_add_default_slice(u, &s->cgroup_context);
                 if (r < 0)
                         return r;
 
@@ -1216,27 +1212,23 @@ static int service_load(Unit *u) {
                         return r;
 #endif
 
-                if (s->bus_name) {
-                        r = unit_watch_bus_name(u, s->bus_name);
-                        if (r < 0)
-                                return r;
-                }
-
                 if (s->type == SERVICE_NOTIFY && s->notify_access == NOTIFY_NONE)
                         s->notify_access = NOTIFY_MAIN;
 
                 if (s->watchdog_usec > 0 && s->notify_access == NOTIFY_NONE)
                         s->notify_access = NOTIFY_MAIN;
 
-                if (UNIT(s)->default_dependencies) {
-                        r = service_add_default_dependencies(s);
+                if (s->bus_name) {
+                        r = unit_watch_bus_name(u, s->bus_name);
                         if (r < 0)
                                 return r;
                 }
 
-                r = unit_exec_context_patch_defaults(u, &s->exec_context);
-                if (r < 0)
-                        return r;
+                if (u->default_dependencies) {
+                        r = service_add_default_dependencies(s);
+                        if (r < 0)
+                                return r;
+                }
         }
 
         return service_verify(s);

src/core/slice.c

@@ -36,24 +36,6 @@ static const UnitActiveState state_translation_table[_SLICE_STATE_MAX] = {
         [SLICE_ACTIVE] = UNIT_ACTIVE
 };
 
-static void slice_init(Unit *u) {
-        Slice *s = SLICE(u);
-
-        assert(u);
-        assert(u->load_state == UNIT_STUB);
-
-        cgroup_context_init(&s->cgroup_context);
-        unit_cgroup_context_init_defaults(u, &s->cgroup_context);
-}
-
-static void slice_done(Unit *u) {
-        Slice *s = SLICE(u);
-
-        assert(u);
-
-        cgroup_context_done(&s->cgroup_context);
-}
-
 static void slice_set_state(Slice *t, SliceState state) {
         SliceState old_state;
         assert(t);
@@ -153,6 +135,10 @@ static int slice_load(Unit *u) {
         /* This is a new unit? Then let's add in some extras */
         if (u->load_state == UNIT_LOADED) {
 
+                r = unit_patch_contexts(u);
+                if (r < 0)
+                        return r;
+
                 r = slice_add_parent_slice(s);
                 if (r < 0)
                         return r;
@@ -287,9 +273,7 @@ const UnitVTable slice_vtable = {
         .no_alias = true,
         .no_instances = true,
 
-        .init = slice_init,
         .load = slice_load,
-        .done = slice_done,
 
         .coldplug = slice_coldplug,
 

src/core/socket.c

@@ -90,13 +90,8 @@ static void socket_init(Unit *u) {
         s->ip_ttl = -1;
         s->mark = -1;
 
-        exec_context_init(&s->exec_context);
         s->exec_context.std_output = u->manager->default_std_output;
         s->exec_context.std_error = u->manager->default_std_error;
-        kill_context_init(&s->kill_context);
-        cgroup_context_init(&s->cgroup_context);
-
-        unit_cgroup_context_init_defaults(u, &s->cgroup_context);
 
         s->control_command_id = _SOCKET_EXEC_COMMAND_INVALID;
 }
@@ -134,8 +129,6 @@ static void socket_done(Unit *u) {
 
         socket_free_ports(s);
 
-        cgroup_context_done(&s->cgroup_context);
-        exec_context_done(&s->exec_context);
         s->exec_runtime = exec_runtime_unref(s->exec_runtime);
         exec_command_free_array(s->exec_command, _SOCKET_EXEC_COMMAND_MAX);
         s->control_command = NULL;
@@ -336,7 +329,7 @@ static int socket_add_extras(Socket *s) {
         if (r < 0)
                 return r;
 
-        r = unit_exec_context_patch_defaults(u, &s->exec_context);
+        r = unit_patch_contexts(u);
         if (r < 0)
                 return r;
 
@@ -345,7 +338,7 @@ static int socket_add_extras(Socket *s) {
                 if (r < 0)
                         return r;
 
-                r = unit_add_default_slice(u);
+                r = unit_add_default_slice(u, &s->cgroup_context);
                 if (r < 0)
                         return r;
         }

src/core/swap.c

@@ -118,13 +118,8 @@ static void swap_init(Unit *u) {
 
         s->timeout_usec = u->manager->default_timeout_start_usec;
 
-        exec_context_init(&s->exec_context);
         s->exec_context.std_output = u->manager->default_std_output;
         s->exec_context.std_error = u->manager->default_std_error;
-        kill_context_init(&s->kill_context);
-        cgroup_context_init(&s->cgroup_context);
-
-        unit_cgroup_context_init_defaults(u, &s->cgroup_context);
 
         s->parameters_proc_swaps.priority = s->parameters_fragment.priority = -1;
 
@@ -157,8 +152,6 @@ static void swap_done(Unit *u) {
         free(s->parameters_fragment.what);
         s->parameters_fragment.what = NULL;
 
-        cgroup_context_done(&s->cgroup_context);
-        exec_context_done(&s->exec_context);
         s->exec_runtime = exec_runtime_unref(s->exec_runtime);
         exec_command_done_array(s->exec_command, _SWAP_EXEC_COMMAND_MAX);
         s->control_command = NULL;
@@ -305,9 +298,6 @@ static int swap_load(Unit *u) {
                 return r;
 
         if (u->load_state == UNIT_LOADED) {
-                r = unit_add_exec_dependencies(u, &s->exec_context);
-                if (r < 0)
-                        return r;
 
                 if (UNIT(s)->fragment_path)
                         s->from_fragment = true;
@@ -344,7 +334,15 @@ static int swap_load(Unit *u) {
                 if (r < 0)
                         return r;
 
-                r = unit_add_default_slice(u);
+                r = unit_patch_contexts(u);
+                if (r < 0)
+                        return r;
+
+                r = unit_add_exec_dependencies(u, &s->exec_context);
+                if (r < 0)
+                        return r;
+
+                r = unit_add_default_slice(u, &s->cgroup_context);
                 if (r < 0)
                         return r;
 
@@ -353,10 +351,6 @@ static int swap_load(Unit *u) {
                         if (r < 0)
                                 return r;
                 }
-
-                r = unit_exec_context_patch_defaults(u, &s->exec_context);
-                if (r < 0)
-                        return r;
         }
 
         return swap_verify(s);

src/core/unit.c

@@ -101,104 +101,122 @@ bool unit_has_name(Unit *u, const char *name) {
         return !!set_get(u->names, (char*) name);
 }
 
+static void unit_init(Unit *u) {
+        CGroupContext *cc;
+        ExecContext *ec;
+        KillContext *kc;
+
+        assert(u);
+        assert(u->manager);
+        assert(u->type >= 0);
+
+        cc = unit_get_cgroup_context(u);
+        if (cc) {
+                cgroup_context_init(cc);
+
+                /* Copy in the manager defaults into the cgroup
+                 * context, _before_ the rest of the settings have
+                 * been initialized */
+
+                cc->cpu_accounting = u->manager->default_cpu_accounting;
+                cc->blockio_accounting = u->manager->default_blockio_accounting;
+                cc->memory_accounting = u->manager->default_memory_accounting;
+        }
+
+        ec = unit_get_exec_context(u);
+        if (ec)
+                exec_context_init(ec);
+
+        kc = unit_get_kill_context(u);
+        if (kc)
+                kill_context_init(kc);
+
+        if (UNIT_VTABLE(u)->init)
+                UNIT_VTABLE(u)->init(u);
+}
+
 int unit_add_name(Unit *u, const char *text) {
+        _cleanup_free_ char *s = NULL, *i = NULL;
         UnitType t;
-        char *s, *i = NULL;
         int r;
 
         assert(u);
         assert(text);
 
         if (unit_name_is_template(text)) {
+
                 if (!u->instance)
                         return -EINVAL;
 
                 s = unit_name_replace_instance(text, u->instance);
         } else
                 s = strdup(text);
-
         if (!s)
                 return -ENOMEM;
 
-        if (!unit_name_is_valid(s, TEMPLATE_INVALID)) {
-                r = -EINVAL;
-                goto fail;
-        }
+        if (!unit_name_is_valid(s, TEMPLATE_INVALID))
+                return -EINVAL;
 
         assert_se((t = unit_name_to_type(s)) >= 0);
 
-        if (u->type != _UNIT_TYPE_INVALID && t != u->type) {
-                r = -EINVAL;
-                goto fail;
-        }
+        if (u->type != _UNIT_TYPE_INVALID && t != u->type)
+                return -EINVAL;
 
         r = unit_name_to_instance(s, &i);
         if (r < 0)
-                goto fail;
+                return r;
 
-        if (i && unit_vtable[t]->no_instances) {
-                r = -EINVAL;
-                goto fail;
-        }
+        if (i && unit_vtable[t]->no_instances)
+                return -EINVAL;
 
         /* Ensure that this unit is either instanced or not instanced,
          * but not both. */
-        if (u->type != _UNIT_TYPE_INVALID && !u->instance != !i) {
-                r = -EINVAL;
-                goto fail;
-        }
+        if (u->type != _UNIT_TYPE_INVALID && !u->instance != !i)
+                return -EINVAL;
 
         if (unit_vtable[t]->no_alias &&
             !set_isempty(u->names) &&
-            !set_get(u->names, s)) {
-                r = -EEXIST;
-                goto fail;
-        }
+            !set_get(u->names, s))
+                return -EEXIST;
 
-        if (hashmap_size(u->manager->units) >= MANAGER_MAX_NAMES) {
-                r = -E2BIG;
-                goto fail;
-        }
+        if (hashmap_size(u->manager->units) >= MANAGER_MAX_NAMES)
+                return -E2BIG;
 
         r = set_put(u->names, s);
         if (r < 0) {
                 if (r == -EEXIST)
-                        r = 0;
-                goto fail;
+                        return 0;
+
+                return r;
         }
 
         r = hashmap_put(u->manager->units, s, u);
         if (r < 0) {
                 set_remove(u->names, s);
-                goto fail;
+                return r;
         }
 
         if (u->type == _UNIT_TYPE_INVALID) {
-
                 u->type = t;
                 u->id = s;
                 u->instance = i;
 
                 LIST_PREPEND(units_by_type, u->manager->units_by_type[t], u);
 
-                if (UNIT_VTABLE(u)->init)
-                        UNIT_VTABLE(u)->init(u);
-        } else
-                free(i);
+                unit_init(u);
+
+                i = NULL;
+        }
+
+        s = NULL;
 
         unit_add_to_dbus_queue(u);
         return 0;
-
-fail:
-        free(s);
-        free(i);
-
-        return r;
 }
 
 int unit_choose_id(Unit *u, const char *name) {
-        char *s, *i;
         _cleanup_free_ char *t = NULL;
+        char *s, *i;
         int r;
 
         assert(u);
@@ -218,7 +236,6 @@ int unit_choose_id(Unit *u, const char *name) {
 
         /* Selects one of the names of this unit as the id */
         s = set_get(u->names, (char*) name);
-
         if (!s)
                 return -ENOENT;
 
@@ -410,6 +427,27 @@ static void unit_free_requires_mounts_for(Unit *u) {
         u->requires_mounts_for = NULL;
 }
 
+static void unit_done(Unit *u) {
+        ExecContext *ec;
+        CGroupContext *cc;
+
+        assert(u);
+
+        if (u->type < 0)
+                return;
+
+        if (UNIT_VTABLE(u)->done)
+                UNIT_VTABLE(u)->done(u);
+
+        ec = unit_get_exec_context(u);
+        if (ec)
+                exec_context_done(ec);
+
+        cc = unit_get_cgroup_context(u);
+        if (cc)
+                cgroup_context_done(cc);
+}
+
 void unit_free(Unit *u) {
         UnitDependency d;
         Iterator i;
@@ -422,9 +460,7 @@ void unit_free(Unit *u) {
 
         bus_unit_send_removed_signal(u);
 
-        if (u->load_state != UNIT_STUB)
-                if (UNIT_VTABLE(u)->done)
-                        UNIT_VTABLE(u)->done(u);
+        unit_done(u);
 
         unit_free_requires_mounts_for(u);
 
@@ -2148,20 +2184,18 @@ char *unit_default_cgroup_path(Unit *u) {
                 return strjoin(u->manager->cgroup_root, "/", escaped, NULL);
 }
 
-int unit_add_default_slice(Unit *u) {
+int unit_add_default_slice(Unit *u, CGroupContext *c) {
         _cleanup_free_ char *b = NULL;
         const char *slice_name;
         Unit *slice;
         int r;
 
         assert(u);
+        assert(c);
 
         if (UNIT_ISSET(u->slice))
                 return 0;
 
-        if (!unit_get_cgroup_context(u))
-                return 0;
-
         if (u->instance) {
                 _cleanup_free_ char *prefix = NULL, *escaped = NULL;
 
@@ -2793,56 +2827,56 @@ void unit_ref_unset(UnitRef *ref) {
         ref->unit = NULL;
 }
 
-int unit_cgroup_context_init_defaults(Unit *u, CGroupContext *c) {
-        assert(u);
-        assert(c);
-
-        /* Copy in the manager defaults into the cgroup context,
-         * _before_ the rest of the settings have been initialized */
-
-        c->cpu_accounting = u->manager->default_cpu_accounting;
-        c->blockio_accounting = u->manager->default_blockio_accounting;
-        c->memory_accounting = u->manager->default_memory_accounting;
-
-        return 0;
-}
-
-int unit_exec_context_patch_defaults(Unit *u, ExecContext *c) {
+int unit_patch_contexts(Unit *u) {
+        CGroupContext *cc;
+        ExecContext *ec;
         unsigned i;
         int r;
 
         assert(u);
-        assert(c);
 
-        /* Patch in the manager defaults into the exec context,
-         * _after_ the rest of the settings have been initialized */
+        /* Patch in the manager defaults into the exec and cgroup
+         * contexts, _after_ the rest of the settings have been
+         * initialized */
 
-        /* This only copies in the ones that need memory */
-        for (i = 0; i < _RLIMIT_MAX; i++)
-                if (u->manager->rlimit[i] && !c->rlimit[i]) {
-                        c->rlimit[i] = newdup(struct rlimit, u->manager->rlimit[i], 1);
-                        if (!c->rlimit[i])
-                                return -ENOMEM;
+        ec = unit_get_exec_context(u);
+        if (ec) {
+                /* This only copies in the ones that need memory */
+                for (i = 0; i < _RLIMIT_MAX; i++)
+                        if (u->manager->rlimit[i] && !ec->rlimit[i]) {
+                                ec->rlimit[i] = newdup(struct rlimit, u->manager->rlimit[i], 1);
+                                if (!ec->rlimit[i])
+                                        return -ENOMEM;
+                        }
+
+                if (u->manager->running_as == SYSTEMD_USER &&
+                    !ec->working_directory) {
+
+                        r = get_home_dir(&ec->working_directory);
+                        if (r < 0)
+                                return r;
                 }
 
-        if (u->manager->running_as == SYSTEMD_USER &&
-            !c->working_directory) {
+                if (u->manager->running_as == SYSTEMD_USER &&
+                    (ec->syscall_whitelist ||
+                     !set_isempty(ec->syscall_filter) ||
+                     !set_isempty(ec->syscall_archs) ||
+                     ec->address_families_whitelist ||
+                     !set_isempty(ec->address_families)))
+                        ec->no_new_privileges = true;
 
-                r = get_home_dir(&c->working_directory);
-                if (r < 0)
-                        return r;
+                if (ec->private_devices)
+                        ec->capability_bounding_set_drop |= (uint64_t) 1ULL << (uint64_t) CAP_MKNOD;
         }
 
-        if (u->manager->running_as == SYSTEMD_USER &&
-            (c->syscall_whitelist ||
-             !set_isempty(c->syscall_filter) ||
-             !set_isempty(c->syscall_archs) ||
-             c->address_families_whitelist ||
-             !set_isempty(c->address_families)))
-                c->no_new_privileges = true;
+        cc = unit_get_cgroup_context(u);
+        if (cc) {
 
-        if (c->private_devices)
-                c->capability_bounding_set_drop |= (uint64_t) 1ULL << (uint64_t) CAP_MKNOD;
+                if (ec &&
+                    ec->private_devices &&
+                    cc->device_policy == CGROUP_AUTO)
+                        cc->device_policy = CGROUP_CLOSED;
+        }
 
         return 0;
 }
@@ -2851,6 +2885,9 @@ ExecContext *unit_get_exec_context(Unit *u) {
         size_t offset;
         assert(u);
 
+        if (u->type < 0)
+                return NULL;
+
         offset = UNIT_VTABLE(u)->exec_context_offset;
         if (offset <= 0)
                 return NULL;
@@ -2862,6 +2899,9 @@ KillContext *unit_get_kill_context(Unit *u) {
         size_t offset;
         assert(u);
 
+        if (u->type < 0)
+                return NULL;
+
         offset = UNIT_VTABLE(u)->kill_context_offset;
         if (offset <= 0)
                 return NULL;
@@ -2872,6 +2912,9 @@ KillContext *unit_get_kill_context(Unit *u) {
 CGroupContext *unit_get_cgroup_context(Unit *u) {
         size_t offset;
 
+        if (u->type < 0)
+                return NULL;
+
         offset = UNIT_VTABLE(u)->cgroup_context_offset;
         if (offset <= 0)
                 return NULL;
@@ -2882,6 +2925,9 @@ CGroupContext *unit_get_cgroup_context(Unit *u) {
 ExecRuntime *unit_get_exec_runtime(Unit *u) {
         size_t offset;
 
+        if (u->type < 0)
+                return NULL;
+
         offset = UNIT_VTABLE(u)->exec_runtime_offset;
         if (offset <= 0)
                 return NULL;

src/core/unit.h

@@ -519,7 +519,7 @@ int unit_load_fragment_and_dropin(Unit *u);
 int unit_load_fragment_and_dropin_optional(Unit *u);
 int unit_load(Unit *unit);
 
-int unit_add_default_slice(Unit *u);
+int unit_add_default_slice(Unit *u, CGroupContext *c);
 
 const char *unit_description(Unit *u) _pure_;
 
@@ -602,12 +602,12 @@ void unit_ref_unset(UnitRef *ref);
 #define UNIT_DEREF(ref) ((ref).unit)
 #define UNIT_ISSET(ref) (!!(ref).unit)
 
-int unit_exec_context_patch_defaults(Unit *u, ExecContext *c);
-int unit_cgroup_context_init_defaults(Unit *u, CGroupContext *c);
+int unit_patch_contexts(Unit *u);
 
 ExecContext *unit_get_exec_context(Unit *u) _pure_;
 KillContext *unit_get_kill_context(Unit *u) _pure_;
 CGroupContext *unit_get_cgroup_context(Unit *u) _pure_;
+
 ExecRuntime *unit_get_exec_runtime(Unit *u) _pure_;
 
 int unit_setup_exec_runtime(Unit *u);