From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Tue, 13 Feb 2018 22:57:43 +0000 (+0100)
Subject: pid1: properly remove references to the unit from gc queue during final cleanup
X-Git-Tag: v238~97^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1bdf2790025e661e41894129eb390bb032b88585;p=thirdparty%2Fsystemd.git

pid1: properly remove references to the unit from gc queue during final cleanup

When various references to the unit were dropped during cleanup in unit_free(),
add_to_gc_queue() could be called on this unit. If the unit was previously in
the gc queue (at the time when unit_free() was called on it), this wouldn't
matter, because it'd have in_gc_queue still set even though it was already
removed from the queue. But if it wasn't set, then the unit could be added to
the queue. Then after unit_free() would deallocate the unit, we would be left
with a dangling pointer in gc_queue.

A unit could be added to the gc queue in two places called from unit_free():
in the job_install calls, and in unit_ref_unset(). The first was OK, because
it was above the LIST_REMOVE(gc_queue,...) call, but the second was not, because
it was after that. Move the all LIST_REMOVE() calls down.
---

diff --git a/src/core/unit.c b/src/core/unit.c
index 15cfe84e2f7..5e619b0b079 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -610,27 +610,6 @@ void unit_free(Unit *u) {
         for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++)
                 bidi_set_free(u, u->dependencies[d]);
 
-        if (u->type != _UNIT_TYPE_INVALID)
-                LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u);
-
-        if (u->in_load_queue)
-                LIST_REMOVE(load_queue, u->manager->load_queue, u);
-
-        if (u->in_dbus_queue)
-                LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u);
-
-        if (u->in_cleanup_queue)
-                LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u);
-
-        if (u->in_gc_queue)
-                LIST_REMOVE(gc_queue, u->manager->gc_unit_queue, u);
-
-        if (u->in_cgroup_realize_queue)
-                LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
-
-        if (u->in_cgroup_empty_queue)
-                LIST_REMOVE(cgroup_empty_queue, u->manager->cgroup_empty_queue, u);
-
         if (u->on_console)
                 manager_unref_console(u->manager);
 
@@ -650,6 +629,27 @@ void unit_free(Unit *u) {
         while (u->refs_by_target)
                 unit_ref_unset(u->refs_by_target);
 
+        if (u->type != _UNIT_TYPE_INVALID)
+                LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u);
+
+        if (u->in_load_queue)
+                LIST_REMOVE(load_queue, u->manager->load_queue, u);
+
+        if (u->in_dbus_queue)
+                LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u);
+
+        if (u->in_gc_queue)
+                LIST_REMOVE(gc_queue, u->manager->gc_unit_queue, u);
+
+        if (u->in_cgroup_realize_queue)
+                LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
+
+        if (u->in_cgroup_empty_queue)
+                LIST_REMOVE(cgroup_empty_queue, u->manager->cgroup_empty_queue, u);
+
+        if (u->in_cleanup_queue)
+                LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u);
+
         safe_close(u->ip_accounting_ingress_map_fd);
         safe_close(u->ip_accounting_egress_map_fd);