diff -Nru systemd-229/debian/changelog systemd-229/debian/changelog --- systemd-229/debian/changelog 2019-03-13 16:17:45.000000000 +0100 +++ systemd-229/debian/changelog 2019-03-15 16:46:49.000000000 +0100 @@ -1,3 +1,10 @@ +systemd (229-4ubuntu21.19) xenial; urgency=medium + + * d/p/fix-race-daemon-reload-8803.patch: + this is really PR#11121 now + + -- Michael Vogt Fri, 15 Mar 2019 16:46:49 +0100 + systemd (229-4ubuntu21.18) xenial; urgency=medium * d/p/fix-race-daemon-reload-8803.patch: diff -Nru systemd-229/debian/patches/fix-race-daemon-reload-11121.patch systemd-229/debian/patches/fix-race-daemon-reload-11121.patch --- systemd-229/debian/patches/fix-race-daemon-reload-11121.patch 1970-01-01 01:00:00.000000000 +0100 +++ systemd-229/debian/patches/fix-race-daemon-reload-11121.patch 2019-03-15 16:46:49.000000000 +0100 @@ -0,0 +1,380 @@ +From: Michael Vogt +Date: Mon, 25 Mar 2019 08:44:34 +0100 +Subject: Backport daemon reload race fix (PR#11121) + +There is a race in the systemd code when daemon-reload happens +in parallel with other systemctl operations (like start/stop). +Upstream fixed this first in PR#8803 but the fix is incomplete +and sometimes causes a segfault (see systemd issue #10716). +The full fix is in PR#11121 and this patch first applied #8803 with +the PR #11121 on top. +Index: systemd-229/src/core/job.c +=================================================================== +--- systemd-229.orig/src/core/job.c ++++ systemd-229/src/core/job.c +@@ -77,7 +77,7 @@ Job* job_new(Unit *unit, JobType type) { + return j; + } + +-void job_free(Job *j) { ++void job_unlink(Job *j) { + assert(j); + assert(!j->installed); + assert(!j->transaction_prev); +@@ -85,18 +85,33 @@ void job_free(Job *j) { + assert(!j->subject_list); + assert(!j->object_list); + +- if (j->in_run_queue) ++ if (j->in_run_queue) { + LIST_REMOVE(run_queue, j->manager->run_queue, j); ++ j->in_run_queue = false; ++ } + +- if (j->in_dbus_queue) ++ if (j->in_dbus_queue) { + LIST_REMOVE(dbus_queue, j->manager->dbus_job_queue, j); ++ j->in_dbus_queue = false; ++ } ++ ++ j->timer_event_source = sd_event_source_unref(j->timer_event_source); ++} + +- sd_event_source_unref(j->timer_event_source); ++Job* job_free(Job *j) { ++ assert(j); ++ assert(!j->installed); ++ assert(!j->transaction_prev); ++ assert(!j->transaction_next); ++ assert(!j->subject_list); ++ assert(!j->object_list); + ++ job_unlink(j); ++ + sd_bus_track_unref(j->clients); + strv_free(j->deserialized_clients); + +- free(j); ++ return mfree(j); + } + + static void job_set_state(Job *j, JobState state) { +@@ -145,7 +160,7 @@ void job_uninstall(Job *j) { + + unit_add_to_gc_queue(j->unit); + +- hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); ++ hashmap_remove_value(j->manager->jobs, UINT32_TO_PTR(j->id), j); + j->installed = false; + } + +@@ -232,6 +247,7 @@ Job* job_install(Job *j) { + + int job_install_deserialized(Job *j) { + Job **pj; ++ int r; + + assert(!j->installed); + +@@ -241,10 +257,15 @@ int job_install_deserialized(Job *j) { + } + + pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; +- if (*pj) { +- log_unit_debug(j->unit, "Unit already has a job installed. Not installing deserialized job."); +- return -EEXIST; +- } ++ if (*pj) ++ return log_unit_debug_errno(j->unit, EEXIST, ++ "Unit already has a job installed. Not installing deserialized job."); ++ ++ r = hashmap_put(j->manager->jobs, UINT32_TO_PTR(j->id), j); ++ if (r == -EEXIST) ++ return log_unit_debug_errno(j->unit, r, "Job ID %" PRIu32 " already used, cannot deserialize job.", j->id); ++ if (r < 0) ++ return log_unit_debug_errno(j->unit, r, "Failed to insert job into jobs hash table: %m"); + + *pj = j; + j->installed = true; +Index: systemd-229/src/core/job.h +=================================================================== +--- systemd-229.orig/src/core/job.h ++++ systemd-229/src/core/job.h +@@ -172,7 +172,8 @@ struct Job { + + Job* job_new(Unit *unit, JobType type); + Job* job_new_raw(Unit *unit); +-void job_free(Job *job); ++void job_unlink(Job *job); ++Job* job_free(Job *job); + Job* job_install(Job *j); + int job_install_deserialized(Job *j); + void job_uninstall(Job *j); +@@ -227,6 +228,8 @@ void job_shutdown_magic(Job *j); + + int job_get_timeout(Job *j, usec_t *timeout) _pure_; + ++DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free); ++ + const char* job_type_to_string(JobType t) _const_; + JobType job_type_from_string(const char *s) _pure_; + +Index: systemd-229/src/core/unit.c +=================================================================== +--- systemd-229.orig/src/core/unit.c ++++ systemd-229/src/core/unit.c +@@ -1811,9 +1811,75 @@ void unit_trigger_notify(Unit *u) { + UNIT_VTABLE(other)->trigger_notify(other, u); + } + ++static bool unit_process_job(Job *j, UnitActiveState ns, bool reload_success) { ++ bool unexpected = false; ++ ++ assert(j); ++ ++ if (j->state == JOB_WAITING) ++ ++ /* So we reached a different state for this job. Let's see if we can run it now if it failed previously ++ * due to EAGAIN. */ ++ job_add_to_run_queue(j); ++ ++ /* Let's check whether the unit's new state constitutes a finished job, or maybe contradicts a running job and ++ * hence needs to invalidate jobs. */ ++ ++ switch (j->type) { ++ ++ case JOB_START: ++ case JOB_VERIFY_ACTIVE: ++ ++ if (UNIT_IS_ACTIVE_OR_RELOADING(ns)) ++ job_finish_and_invalidate(j, JOB_DONE, true); ++ else if (j->state == JOB_RUNNING && ns != UNIT_ACTIVATING) { ++ unexpected = true; ++ ++ if (UNIT_IS_INACTIVE_OR_FAILED(ns)) ++ job_finish_and_invalidate(j, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); ++ } ++ ++ break; ++ ++ case JOB_RELOAD: ++ case JOB_RELOAD_OR_START: ++ case JOB_TRY_RELOAD: ++ ++ if (j->state == JOB_RUNNING) { ++ if (ns == UNIT_ACTIVE) ++- job_finish_and_invalidate(j, reload_success ? JOB_DONE : JOB_FAILED, true); ++ else if (!IN_SET(ns, UNIT_ACTIVATING, UNIT_RELOADING)) { ++ unexpected = true; ++ ++ if (UNIT_IS_INACTIVE_OR_FAILED(ns)) ++- job_finish_and_invalidate(j, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); ++ } ++ } ++ ++ break; ++ ++ case JOB_STOP: ++ case JOB_RESTART: ++ case JOB_TRY_RESTART: ++ ++ if (UNIT_IS_INACTIVE_OR_FAILED(ns)) ++ job_finish_and_invalidate(j, JOB_DONE, true); ++ else if (j->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) { ++ unexpected = true; ++ job_finish_and_invalidate(j, JOB_FAILED, true); ++ } ++ ++ break; ++ ++ default: ++ assert_not_reached("Job type unknown"); ++ } ++ ++ return unexpected; ++} ++ + void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_success) { + Manager *m; +- bool unexpected; + + assert(u); + assert(os < _UNIT_ACTIVE_STATE_MAX); +@@ -1868,82 +1934,18 @@ void unit_notify(Unit *u, UnitActiveStat + } + } + +- if (u->job) { +- unexpected = false; +- +- if (u->job->state == JOB_WAITING) +- +- /* So we reached a different state for this +- * job. Let's see if we can run it now if it +- * failed previously due to EAGAIN. */ +- job_add_to_run_queue(u->job); +- +- /* Let's check whether this state change constitutes a +- * finished job, or maybe contradicts a running job and +- * hence needs to invalidate jobs. */ +- +- switch (u->job->type) { +- +- case JOB_START: +- case JOB_VERIFY_ACTIVE: +- +- if (UNIT_IS_ACTIVE_OR_RELOADING(ns)) +- job_finish_and_invalidate(u->job, JOB_DONE, true); +- else if (u->job->state == JOB_RUNNING && ns != UNIT_ACTIVATING) { +- unexpected = true; +- +- if (UNIT_IS_INACTIVE_OR_FAILED(ns)) +- job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); +- } +- +- break; +- +- case JOB_RELOAD: +- case JOB_RELOAD_OR_START: +- case JOB_TRY_RELOAD: +- +- if (u->job->state == JOB_RUNNING) { +- if (ns == UNIT_ACTIVE) +- job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true); +- else if (ns != UNIT_ACTIVATING && ns != UNIT_RELOADING) { +- unexpected = true; +- +- if (UNIT_IS_INACTIVE_OR_FAILED(ns)) +- job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); +- } +- } +- +- break; +- +- case JOB_STOP: +- case JOB_RESTART: +- case JOB_TRY_RESTART: +- +- if (UNIT_IS_INACTIVE_OR_FAILED(ns)) +- job_finish_and_invalidate(u->job, JOB_DONE, true); +- else if (u->job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) { +- unexpected = true; +- job_finish_and_invalidate(u->job, JOB_FAILED, true); +- } +- +- break; +- +- default: +- assert_not_reached("Job type unknown"); +- } +- +- } else +- unexpected = true; +- + if (m->n_reloading <= 0) { ++ bool unexpected; + +- /* If this state change happened without being +- * requested by a job, then let's retroactively start +- * or stop dependencies. We skip that step when +- * deserializing, since we don't want to create any +- * additional jobs just because something is already +- * activated. */ ++ /* Let's propagate state changes to the job */ ++ if (u->job) ++ unexpected = unit_process_job(u->job, ns, reload_success); ++ else ++ unexpected = true; + ++ /* If this state change happened without being requested by a job, then let's retroactively start or ++ * stop dependencies. We skip that step when deserializing, since we don't want to create any ++ * additional jobs just because something is already activated. */ + if (unexpected) { + if (UNIT_IS_INACTIVE_OR_FAILED(os) && UNIT_IS_ACTIVE_OR_ACTIVATING(ns)) + retroactively_start_dependencies(u); +@@ -2673,6 +2675,29 @@ void unit_serialize_item_format(Unit *u, + fputc('\n', f); + } + ++static int unit_deserialize_job(Unit *u, FILE *f, FDSet *fds) { ++ _cleanup_(job_freep) Job *j = NULL; ++ int r; ++ ++ assert(u); ++ assert(f); ++ ++ j = job_new_raw(u); ++ if (!j) ++ return log_oom(); ++ ++ r = job_deserialize(j, f, fds); ++ if (r < 0) ++ return r; ++ ++ r = job_install_deserialized(j); ++ if (r < 0) ++ return r; ++ ++ TAKE_PTR(j); ++ return 0; ++} ++ + int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { + ExecRuntime **rt = NULL; + size_t offset; +@@ -2710,33 +2735,12 @@ int unit_deserialize(Unit *u, FILE *f, F + v = l+k; + + if (streq(l, "job")) { +- if (v[0] == '\0') { +- /* new-style serialized job */ +- Job *j; +- +- j = job_new_raw(u); +- if (!j) +- return log_oom(); +- +- r = job_deserialize(j, f, fds); +- if (r < 0) { +- job_free(j); +- return r; +- } +- +- r = hashmap_put(u->manager->jobs, UINT32_TO_PTR(j->id), j); +- if (r < 0) { +- job_free(j); +- return r; +- } +- +- r = job_install_deserialized(j); +- if (r < 0) { +- hashmap_remove(u->manager->jobs, UINT32_TO_PTR(j->id)); +- job_free(j); ++ if (v[0] == '\0') { ++ /* New-style serialized job */ ++ r = unit_deserialize_job(u, f, fds); ++ if (r < 0) + return r; +- } +- } else /* legacy for pre-44 */ ++ } else /* Legacy for pre-44 */ + log_unit_warning(u, "Update from too old systemd versions are unsupported, cannot deserialize job: %s", v); + continue; + } else if (streq(l, "state-change-timestamp")) { +Index: systemd-229/src/basic/alloc-util.h +=================================================================== +--- systemd-229.orig/src/basic/alloc-util.h ++++ systemd-229/src/basic/alloc-util.h +@@ -113,3 +113,12 @@ void* greedy_realloc0(void **p, size_t * + _new_ = alloca_align(_size_, (align)); \ + (void*)memset(_new_, 0, _size_); \ + }) ++ ++/* Takes inspiration from Rusts's Option::take() method: reads and returns a pointer, but at the same time resets it to ++ * NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */ ++#define TAKE_PTR(ptr) \ ++ ({ \ ++ typeof(ptr) _ptr_ = (ptr); \ ++ (ptr) = NULL; \ ++ _ptr_; \ ++ }) diff -Nru systemd-229/debian/patches/fix-race-daemon-reload-8803.patch systemd-229/debian/patches/fix-race-daemon-reload-8803.patch --- systemd-229/debian/patches/fix-race-daemon-reload-8803.patch 2019-03-13 14:56:09.000000000 +0100 +++ systemd-229/debian/patches/fix-race-daemon-reload-8803.patch 1970-01-01 01:00:00.000000000 +0100 @@ -1,170 +0,0 @@ -Index: systemd-229/src/core/job.c -=================================================================== ---- systemd-229.orig/src/core/job.c -+++ systemd-229/src/core/job.c -@@ -56,6 +56,7 @@ Job* job_new_raw(Unit *unit) { - j->manager = unit->manager; - j->unit = unit; - j->type = _JOB_TYPE_INVALID; -+ j->reloaded = false; - - return j; - } -@@ -77,7 +78,7 @@ Job* job_new(Unit *unit, JobType type) { - return j; - } - --void job_free(Job *j) { -+void job_unlink(Job *j) { - assert(j); - assert(!j->installed); - assert(!j->transaction_prev); -@@ -85,14 +86,29 @@ void job_free(Job *j) { - assert(!j->subject_list); - assert(!j->object_list); - -- if (j->in_run_queue) -+ if (j->in_run_queue) { - LIST_REMOVE(run_queue, j->manager->run_queue, j); -+ j->in_run_queue = false; -+ } - -- if (j->in_dbus_queue) -+ if (j->in_dbus_queue) { - LIST_REMOVE(dbus_queue, j->manager->dbus_job_queue, j); -+ j->in_dbus_queue = false; -+ } -+ -+ j->timer_event_source = sd_event_source_unref(j->timer_event_source); -+} - -- sd_event_source_unref(j->timer_event_source); -+void job_free(Job *j) { -+ assert(j); -+ assert(!j->installed); -+ assert(!j->transaction_prev); -+ assert(!j->transaction_next); -+ assert(!j->subject_list); -+ assert(!j->object_list); - -+ job_unlink(j); -+ - sd_bus_track_unref(j->clients); - strv_free(j->deserialized_clients); - -@@ -248,6 +264,7 @@ int job_install_deserialized(Job *j) { - - *pj = j; - j->installed = true; -+ j->reloaded = true; - - if (j->state == JOB_RUNNING) - j->unit->manager->n_running_jobs++; -@@ -832,6 +849,19 @@ static void job_fail_dependencies(Unit * - } - } - -+static int job_save_pending_finished_job(Job *j) { -+ int r; -+ -+ assert(j); -+ -+ r = set_ensure_allocated(&j->manager->pending_finished_jobs, NULL); -+ if (r < 0) -+ return r; -+ -+ job_unlink(j); -+ return set_put(j->manager->pending_finished_jobs, j); -+} -+ - int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) { - Unit *u; - Unit *other; -@@ -868,7 +898,12 @@ int job_finish_and_invalidate(Job *j, Jo - j->manager->n_failed_jobs ++; - - job_uninstall(j); -- job_free(j); -+ /* Remember jobs started before the reload */ -+ if (MANAGER_IS_RELOADING(j->manager) && j->reloaded) { -+ if (job_save_pending_finished_job(j) < 0) -+ job_free(j); -+ } else -+ job_free(j); - - /* Fail depending jobs on failure */ - if (result != JOB_DONE && recursive) { -@@ -1258,3 +1293,4 @@ const char* job_type_to_access_method(Jo - else - return "reload"; - } -+ -Index: systemd-229/src/core/job.h -=================================================================== ---- systemd-229.orig/src/core/job.h -+++ systemd-229/src/core/job.h -@@ -168,10 +168,12 @@ struct Job { - bool sent_dbus_new_signal:1; - bool ignore_order:1; - bool irreversible:1; -+ bool reloaded:1; - }; - - Job* job_new(Unit *unit, JobType type); - Job* job_new_raw(Unit *unit); -+void job_unlink(Job *job); - void job_free(Job *job); - Job* job_install(Job *j); - int job_install_deserialized(Job *j); -Index: systemd-229/src/core/manager.c -=================================================================== ---- systemd-229.orig/src/core/manager.c -+++ systemd-229/src/core/manager.c -@@ -2497,6 +2497,17 @@ finish: - return r; - } - -+static void manager_flush_finished_jobs(Manager *m) { -+ Job *j; -+ -+ while ((j = set_steal_first(m->pending_finished_jobs))) { -+ bus_job_send_removed_signal(j); -+ job_free(j); -+ } -+ -+ m->pending_finished_jobs = set_free(m->pending_finished_jobs); -+} -+ - int manager_reload(Manager *m) { - int r, q; - _cleanup_fclose_ FILE *f = NULL; -@@ -2575,6 +2586,9 @@ int manager_reload(Manager *m) { - assert(m->n_reloading > 0); - m->n_reloading--; - -+ if (!MANAGER_IS_RELOADING(m)) -+ manager_flush_finished_jobs(m); -+ - m->send_reloading_done = true; - - return r; -Index: systemd-229/src/core/manager.h -=================================================================== ---- systemd-229.orig/src/core/manager.h -+++ systemd-229/src/core/manager.h -@@ -266,6 +266,9 @@ struct Manager { - - /* non-zero if we are reloading or reexecuting, */ - int n_reloading; -+ /* A set which contains all jobs that started before reload and finished -+ * during it */ -+ Set *pending_finished_jobs; - - unsigned n_installed_jobs; - unsigned n_failed_jobs; -@@ -374,3 +377,5 @@ int manager_update_failed_units(Manager - - const char *manager_state_to_string(ManagerState m) _const_; - ManagerState manager_state_from_string(const char *s) _pure_; -+ -+#define MANAGER_IS_RELOADING(m) ((m)->n_reloading > 0) diff -Nru systemd-229/debian/patches/series systemd-229/debian/patches/series --- systemd-229/debian/patches/series 2019-03-13 11:25:56.000000000 +0100 +++ systemd-229/debian/patches/series 2019-03-15 16:46:49.000000000 +0100 @@ -134,4 +134,4 @@ journal-do-not-remove-multiple-spaces-after-identifi.patch stop-mount-error-propagation.patch fix-egde-case-when-processing-proc-self-mountinfo.patch -fix-race-daemon-reload-8803.patch +fix-race-daemon-reload-11121.patch