661545
From 8f1df942e2237124f7559176081af7ac631d3422 Mon Sep 17 00:00:00 2001
9ab0c5
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
9ab0c5
Date: Tue, 13 Feb 2018 23:57:43 +0100
9ab0c5
Subject: [PATCH] pid1: properly remove references to the unit from gc queue
9ab0c5
 during final cleanup
9ab0c5
9ab0c5
When various references to the unit were dropped during cleanup in unit_free(),
9ab0c5
add_to_gc_queue() could be called on this unit. If the unit was previously in
9ab0c5
the gc queue (at the time when unit_free() was called on it), this wouldn't
9ab0c5
matter, because it'd have in_gc_queue still set even though it was already
9ab0c5
removed from the queue. But if it wasn't set, then the unit could be added to
9ab0c5
the queue. Then after unit_free() would deallocate the unit, we would be left
9ab0c5
with a dangling pointer in gc_queue.
9ab0c5
9ab0c5
A unit could be added to the gc queue in two places called from unit_free():
9ab0c5
in the job_install calls, and in unit_ref_unset(). The first was OK, because
9ab0c5
it was above the LIST_REMOVE(gc_queue,...) call, but the second was not, because
9ab0c5
it was after that. Move the all LIST_REMOVE() calls down.
9ab0c5
9ab0c5
(cherry picked from commit 1bdf2790025e661e41894129eb390bb032b88585)
9ab0c5
661545
Related: #1718953
9ab0c5
---
9ab0c5
 src/core/unit.c | 34 +++++++++++++++++-----------------
9ab0c5
 1 file changed, 17 insertions(+), 17 deletions(-)
9ab0c5
9ab0c5
diff --git a/src/core/unit.c b/src/core/unit.c
9ab0c5
index 63f00acc0a..def36a0930 100644
9ab0c5
--- a/src/core/unit.c
9ab0c5
+++ b/src/core/unit.c
9ab0c5
@@ -506,23 +506,6 @@ void unit_free(Unit *u) {
9ab0c5
         for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++)
9ab0c5
                 bidi_set_free(u, u->dependencies[d]);
9ab0c5
 
9ab0c5
-        if (u->type != _UNIT_TYPE_INVALID)
9ab0c5
-                LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u);
9ab0c5
-
9ab0c5
-        if (u->in_load_queue)
9ab0c5
-                LIST_REMOVE(load_queue, u->manager->load_queue, u);
9ab0c5
-
9ab0c5
-        if (u->in_dbus_queue)
9ab0c5
-                LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u);
9ab0c5
-
9ab0c5
-        if (u->in_cleanup_queue)
9ab0c5
-                LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u);
9ab0c5
-
9ab0c5
-        if (u->in_gc_queue) {
9ab0c5
-                LIST_REMOVE(gc_queue, u->manager->gc_queue, u);
9ab0c5
-                u->manager->n_in_gc_queue--;
9ab0c5
-        }
9ab0c5
-
9ab0c5
         if (u->in_target_deps_queue)
9ab0c5
                 LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
9ab0c5
 
9ab0c5
@@ -543,6 +526,23 @@ void unit_free(Unit *u) {
9ab0c5
         while (u->refs_by_target)
9ab0c5
                 unit_ref_unset(u->refs_by_target);
9ab0c5
 
9ab0c5
+        if (u->type != _UNIT_TYPE_INVALID)
9ab0c5
+                LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u);
9ab0c5
+
9ab0c5
+        if (u->in_load_queue)
9ab0c5
+                LIST_REMOVE(load_queue, u->manager->load_queue, u);
9ab0c5
+
9ab0c5
+        if (u->in_dbus_queue)
9ab0c5
+                LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u);
9ab0c5
+
9ab0c5
+        if (u->in_cleanup_queue)
9ab0c5
+                LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u);
9ab0c5
+
9ab0c5
+        if (u->in_gc_queue) {
9ab0c5
+                LIST_REMOVE(gc_queue, u->manager->gc_queue, u);
9ab0c5
+                u->manager->n_in_gc_queue--;
9ab0c5
+        }
9ab0c5
+
9ab0c5
         condition_free_list(u->conditions);
9ab0c5
         condition_free_list(u->asserts);
9ab0c5