URL: https://github.com/SSSD/sssd/pull/934 Author: alexey-tikhonov Title: #934: SBUS: defer deallocation of sbus_watch_ctx Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/934/head:pr934 git checkout pr934
From 7b541022560fd0d232857f16bb77ef177c1b7622 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov <[email protected]> Date: Mon, 11 Nov 2019 18:01:50 +0100 Subject: [PATCH] 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 --- src/sbus/connection/sbus_watch.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/sbus/connection/sbus_watch.c b/src/sbus/connection/sbus_watch.c index 0e4bd01d10..3abb66fa47 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; @@ -253,6 +261,14 @@ sbus_watch_add(DBusWatch *dbus_watch, void *data) return TRUE; } +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) { @@ -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); } }
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/[email protected]
