From f845355e32127c5e8f2bf700cdaa5b8721804232 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Fri, 8 Nov 2019 20:01:50 +0100
Subject: [PATCH] SBUS: defer deallocation of sbus_watch_ctx
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The following flow was causing use-after-free error:
  tevent_common_invoke_fd_handler(RW) -> sbus_watch_handler(RW) ->
  dbus_watch_handle(R) -> ...libdbus detects connection is closed... ->
  sbus_remove_watch() -> talloc_free(watch) ->
  ... get back to libdbus and back to sbus_watch_handler() ->
  "if (watch->dbus_write_watch) dbus_watch_handle(W)" => use-after-free

To resolve an issue schedule deallocation of watch as immediate event.

Resolves: https://pagure.io/SSSD/sssd/issue/2660

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
---
 src/sbus/sssd_dbus_common.c  | 24 +++++++++++++++++++++++-
 src/sbus/sssd_dbus_private.h |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/sbus/sssd_dbus_common.c b/src/sbus/sssd_dbus_common.c
index 50100320a..dbdcae9ec 100644
--- a/src/sbus/sssd_dbus_common.c
+++ b/src/sbus/sssd_dbus_common.c
@@ -133,6 +133,12 @@ dbus_bool_t sbus_add_watch(DBusWatch *dbus_watch, void *data)
             DEBUG(SSSDBG_FATAL_FAILURE, "Out of Memory!\n");
             return FALSE;
         }
+        watch->im_event = tevent_create_immediate(watch);
+        if (watch->im_event == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "Out of Memory!\n");
+            talloc_free(watch);
+            return FALSE;
+        }
         watch->conn = conn;
         watch->fd = fd;
     }
@@ -243,6 +249,13 @@ void sbus_toggle_watch(DBusWatch *dbus_watch, void *data)
            enabled?"enabled":"disabled");
 }
 
+static void free_sbus_watch(struct tevent_context *ev,
+                            struct tevent_immediate *im,
+                            void *data)
+{
+    struct sbus_watch_ctx *w = talloc_get_type(data, struct sbus_watch_ctx);
+    talloc_free(w); /* this will free attached 'im' as well */
+}
 /*
  * sbus_remove_watch
  * Hook for D-BUS to remove file descriptor-based events
@@ -274,7 +287,16 @@ void sbus_remove_watch(DBusWatch *dbus_watch, void *data)
         watch->dbus_write_watch = NULL;
     }
     if (!watch->dbus_read_watch && !watch->dbus_write_watch) {
-        talloc_free(watch);
+        /* libdus doesn't need this watch{fd} anymore, so associated
+         * tevent_fd should be removed from monitoring at the spot.
+         */
+        talloc_zfree(watch->fde);
+        /* watch itself can't be freed yet as it still may be referenced
+         * in the current context (for example in sbus_watch_handler())
+         * so instead schedule immediate event to delete it.
+         */
+        tevent_schedule_immediate(watch->im_event, watch->conn->ev,
+                                  free_sbus_watch, watch);
     }
 }
 
diff --git a/src/sbus/sssd_dbus_private.h b/src/sbus/sssd_dbus_private.h
index a3d4bae16..92649f113 100644
--- a/src/sbus/sssd_dbus_private.h
+++ b/src/sbus/sssd_dbus_private.h
@@ -88,6 +88,7 @@ struct sbus_watch_ctx {
 
     struct tevent_fd *fde;
     int fd;
+    struct tevent_immediate *im_event;
 
     DBusWatch *dbus_read_watch;
     DBusWatch *dbus_write_watch;
-- 
2.21.1