Merge pull request #18915 from keszybz/reexec-bug

Fix crash during daemon-reexec with systemd-oomd running

src/core/core-varlink.c

@@ -142,7 +142,7 @@ static int vl_method_subscribe_managed_oom_cgroups(
         /* We only take one subscriber for this method so return an error if there's already an existing one.
          * This shouldn't happen since systemd-oomd is the only client of this method. */
         if (FLAGS_SET(flags, VARLINK_METHOD_MORE) && m->managed_oom_varlink_request)
-                return varlink_error(m->managed_oom_varlink_request, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL);
+                return varlink_error(link, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL);
 
         r = json_build(&arr, JSON_BUILD_EMPTY_ARRAY);
         if (r < 0)
@@ -188,6 +188,7 @@ static int vl_method_subscribe_managed_oom_cgroups(
         if (!FLAGS_SET(flags, VARLINK_METHOD_MORE))
                 return varlink_reply(link, v);
 
+        assert(!m->managed_oom_varlink_request);
         m->managed_oom_varlink_request = varlink_ref(link);
         return varlink_notify(m->managed_oom_varlink_request, v);
 }
@@ -475,8 +476,7 @@ void manager_varlink_done(Manager *m) {
         assert(m);
 
         /* Send the final message if we still have a subscribe request open. */
-        if (m->managed_oom_varlink_request)
-                m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request);
+        m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request);
 
         m->varlink_server = varlink_server_unref(m->varlink_server);
 }

src/oom/oomd.c

@@ -170,7 +170,7 @@ static int run(int argc, char *argv[]) {
 
         notify_msg = notify_start(NOTIFY_READY, NOTIFY_STOPPING);
 
-        log_info("systemd-oomd starting%s!", arg_dry_run ? " in dry run mode" : "");
+        log_debug("systemd-oomd started%s.", arg_dry_run ? " in dry run mode" : "");
 
         r = sd_event_loop(m->event);
         if (r < 0)

src/shared/varlink.c

@@ -1206,8 +1206,9 @@ int varlink_close(Varlink *v) {
 
         varlink_set_state(v, VARLINK_DISCONNECTED);
 
-        /* Let's take a reference first, since varlink_detach_server() might drop the final (dangling) ref
-         * which would destroy us before we can call varlink_clear() */
+        /* Let's take a reference first, since varlink_detach_server() might drop the final ref from the
+         * disconnect callback, which would invalidate the pointer we are holding before we can call
+         * varlink_clear(). */
         varlink_ref(v);
         varlink_detach_server(v);
         varlink_clear(v);
@@ -1220,17 +1221,33 @@ Varlink* varlink_close_unref(Varlink *v) {
         if (!v)
                 return NULL;
 
-        (void) varlink_close(v);
+        /* A reference is given to us to be destroyed. But when calling varlink_close(), a callback might
+         * also drop a reference. We allow this, and will hold a temporary reference to the object to make
+         * sure that the object still exists when control returns to us. If there's just one reference
+         * remaining after varlink_close(), even though there were at least two right before, we'll handle
+         * that gracefully instead of crashing.
+         *
+         * In other words, this call drops the donated reference, but if the internal call to varlink_close()
+         * dropped a reference to, we don't drop the reference afain. This allows the caller to say:
+         *     global_object->varlink = varlink_close_unref(global_object->varlink);
+         * even though there is some callback which has access to global_object and may drop the reference
+         * stored in global_object->varlink. Without this step, the same code would have to be written as:
+         *     Varlink *t = TAKE_PTR(global_object->varlink);
+         *     varlink_close_unref(t);
+         */
+                                   /* n_ref >= 1 */
+        varlink_ref(v);            /* n_ref >= 2 */
+        varlink_close(v);          /* n_ref >= 1 */
+        if (v->n_ref > 1)
+                v->n_ref--;        /* n_ref >= 1 */
         return varlink_unref(v);
 }
 
 Varlink* varlink_flush_close_unref(Varlink *v) {
-        if (!v)
-                return NULL;
+        if (v)
+                varlink_flush(v);
 
-        (void) varlink_flush(v);
-        (void) varlink_close(v);
-        return varlink_unref(v);
+        return varlink_close_unref(v);
 }
 
 static int varlink_enqueue_json(Varlink *v, JsonVariant *m) {