From d1189c9f92fb79c9623a683a430d2f0f471bf433 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 22 Jan 2020 17:07:47 +0100
Subject: [PATCH 3/3] polkit: when authorizing via PK let's re-resolve
 callback/userdata instead of caching it

Previously, when doing an async PK query we'd store the original
callback/userdata pair and call it again after the PK request is
complete. This is problematic, since PK queries might be slow and in the
meantime the userdata might be released and re-acquired. Let's avoid
this by always traversing through the message handlers so that we always
re-resolve the callback and userdata pair and thus can be sure it's
up-to-date and properly valid.

[fbui: adjust context]
[fbui: sd_event_source_disable_unref() is not available]
[fbui: fixes bsc#1162108]
---
 src/shared/bus-util.c | 78 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c
index 3fb214917f..e654546577 100644
--- a/src/shared/bus-util.c
+++ b/src/shared/bus-util.c
@@ -341,14 +341,13 @@ typedef struct AsyncPolkitQuery {
         char **details;
 
         sd_bus_message *request, *reply;
-        sd_bus_message_handler_t callback;
-        void *userdata;
         sd_bus_slot *slot;
+
         Hashmap *registry;
+        sd_event_source *defer_event_source;
 } AsyncPolkitQuery;
 
 static void async_polkit_query_free(AsyncPolkitQuery *q) {
-
         if (!q)
                 return;
 
@@ -363,9 +362,23 @@ static void async_polkit_query_free(AsyncPolkitQuery *q) {
         free(q->action);
         strv_free(q->details);
 
+        sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_OFF);
+        sd_event_source_unref(q->defer_event_source);
         free(q);
 }
 
+static int async_polkit_defer(sd_event_source *s, void *userdata) {
+        AsyncPolkitQuery *q = userdata;
+
+        assert(s);
+
+        /* This is called as idle event source after we processed the async polkit reply, hopefully after the
+         * method call we re-enqueued has been properly processed. */
+
+        async_polkit_query_free(q);
+        return 0;
+}
+
 static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) {
         _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL;
         AsyncPolkitQuery *q = userdata;
@@ -374,21 +387,46 @@ static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_e
         assert(reply);
         assert(q);
 
+        assert(q->slot);
         q->slot = sd_bus_slot_unref(q->slot);
+
+        assert(!q->reply);
         q->reply = sd_bus_message_ref(reply);
 
+        /* Now, let's dispatch the original message a second time be re-enqueing. This will then traverse the
+         * whole message processing again, and thus re-validating and re-retrieving the "userdata" field
+         * again.
+         *
+         * We install an idle event loop event to clean-up the PolicyKit request data when we are idle again,
+         * i.e. after the second time the message is processed is complete. */
+
+        assert(!q->defer_event_source);
+        r = sd_event_add_defer(sd_bus_get_event(sd_bus_message_get_bus(reply)), &q->defer_event_source, async_polkit_defer, q);
+        if (r < 0)
+                goto fail;
+
+        r = sd_event_source_set_priority(q->defer_event_source, SD_EVENT_PRIORITY_IDLE);
+        if (r < 0)
+                goto fail;
+
+        r = sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_ONESHOT);
+        if (r < 0)
+                goto fail;
+
         r = sd_bus_message_rewind(q->request, true);
-        if (r < 0) {
-                r = sd_bus_reply_method_errno(q->request, r, NULL);
-                goto finish;
-        }
+        if (r < 0)
+                goto fail;
 
-        r = q->callback(q->request, q->userdata, &error_buffer);
-        r = bus_maybe_reply_error(q->request, r, &error_buffer);
+        r = sd_bus_enqueue_for_read(sd_bus_message_get_bus(q->request), q->request);
+        if (r < 0)
+                goto fail;
 
-finish:
-        async_polkit_query_free(q);
+        return 1;
 
+fail:
+        log_debug_errno(r, "Processing asynchronous PolicyKit reply failed, ignoring: %m");
+        (void) sd_bus_reply_method_errno(q->request, r, NULL);
+        async_polkit_query_free(q);
         return r;
 }
 
@@ -407,11 +445,10 @@ int bus_verify_polkit_async(
 #ifdef ENABLE_POLKIT
         _cleanup_bus_message_unref_ sd_bus_message *pk = NULL;
         AsyncPolkitQuery *q;
-        const char *sender, **k, **v;
-        sd_bus_message_handler_t callback;
-        void *userdata;
+        const char **k, **v;
         int c;
 #endif
+        const char *sender;
         int r;
 
         assert(call);
@@ -475,20 +512,11 @@ int bus_verify_polkit_async(
         else if (r > 0)
                 return 1;
 
-#ifdef ENABLE_POLKIT
-        if (sd_bus_get_current_message(call->bus) != call)
-                return -EINVAL;
-
-        callback = sd_bus_get_current_handler(call->bus);
-        if (!callback)
-                return -EINVAL;
-
-        userdata = sd_bus_get_current_userdata(call->bus);
-
         sender = sd_bus_message_get_sender(call);
         if (!sender)
                 return -EBADMSG;
 
+#ifdef ENABLE_POLKIT
         c = sd_bus_message_get_allow_interactive_authorization(call);
         if (c < 0)
                 return c;
@@ -540,8 +568,6 @@ int bus_verify_polkit_async(
                 return -ENOMEM;
 
         q->request = sd_bus_message_ref(call);
-        q->callback = callback;
-        q->userdata = userdata;
 
         q->action = strdup(action);
         if (!q->action) {
-- 
2.16.4

