From f845355e32127c5e8f2bf700cdaa5b8721804232 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov 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 --- 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