From 337a1adf78beefb5dde93a733ab2caf86d65edb9 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Nov 27 2019 10:22:39 +0000
Subject: SBUS: defer deallocation of sbus_watch_ctx


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_watch_remove() -> talloc_free(watch_fd) ->
  ... get back to libdbus and back to sbus_watch_handler() ->
  "if (watch_fd->dbus_watch.write != NULL) 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>

---

diff --git a/src/sbus/connection/sbus_watch.c b/src/sbus/connection/sbus_watch.c
index 0e4bd01..3abb66f 100644
--- a/src/sbus/connection/sbus_watch.c
+++ b/src/sbus/connection/sbus_watch.c
@@ -102,6 +102,7 @@ struct sbus_watch_fd {
 
     int fd;
     struct tevent_fd *fdevent;
+    struct tevent_immediate *im_event;
 
     struct sbus_watch_fd *prev;
     struct sbus_watch_fd *next;
@@ -177,6 +178,13 @@ sbus_watch_get_by_fd(TALLOC_CTX *mem_ctx,
         return NULL;
     }
 
+    watch_fd->im_event = tevent_create_immediate(watch_fd);
+    if (watch_fd->im_event == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of Memory!\n");
+        talloc_free(watch_fd);
+        return NULL;
+    }
+
     talloc_set_destructor(watch_fd, sbus_watch_fd_destructor);
 
     watch_fd->sbus_watch = watch;
@@ -254,6 +262,14 @@ sbus_watch_add(DBusWatch *dbus_watch, void *data)
 }
 
 static void
+free_sbus_watch(struct tevent_context *ev, struct tevent_immediate *im,
+                void *data)
+{
+    struct sbus_watch_fd *w = talloc_get_type(data, struct sbus_watch_fd);
+    talloc_free(w); /* this will free attached 'im' as well */
+}
+
+static void
 sbus_watch_remove(DBusWatch *dbus_watch, void *data)
 {
     struct sbus_watch_fd *watch_fd;
@@ -280,8 +296,16 @@ sbus_watch_remove(DBusWatch *dbus_watch, void *data)
 
     if (watch_fd->dbus_watch.read == NULL
             && watch_fd->dbus_watch.write == NULL) {
-        talloc_free(watch_fd->fdevent);
-        talloc_free(watch_fd);
+        /* libdbus doesn't need this watch{fd} anymore, so associated
+         * tevent_fd should be removed from monitoring at the spot.
+         */
+        talloc_zfree(watch_fd->fdevent);
+        /* watch_fd 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_fd->im_event, watch_fd->sbus_watch->ev,
+                                  free_sbus_watch, watch_fd);
     }
 }