[SSSD] [PATCH] LDAP: Fix crash when processing nested groups
Jakub Hrozek
jhrozek at redhat.com
Wed Aug 7 09:59:26 UTC 2013
On Wed, Aug 07, 2013 at 09:53:13AM +0200, Pavel Březina wrote:
> On 08/06/2013 09:25 PM, Jakub Hrozek wrote:
> >https://fedorahosted.org/sssd/ticket/1932
> >
> >There is a rather strange workaround in the nested groups processing
> >code that calls tevent_req_post outside _send(). However, it broke in
> >certain situations where the tevent_req_call resulted in req being freed,
> >which freed state by extension and then the subsequent _post call was a
> >use-after-free. This patch saves the two variables used outside state so
> >that it's safe to use them even after the callback.
>
> Hi,
> great catch! Sometimes I wish tevent_req_done() would schedule
> immediate event, instead of firing the callback directly.
>
> Nack to the patch but ack to the solution. The same code is also in
> sdap_nested_group_lookup_user(). Can you fix it as well? It maybe
> can't be triggered, but who knows...
I think it can be triggered if all users are out of all search bases.
New patch is attached.
-------------- next part --------------
>From 2a040f571df33894e76ea9fa83119b8f70f44aae Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 6 Aug 2013 21:15:42 +0200
Subject: [PATCH] LDAP: Fix crash when processing nested groups
https://fedorahosted.org/sssd/ticket/1932
There is a rather strange workaround in the nested groups processing
code that calls tevent_req_post outside _send(). However, it broke in
certain situations where the tevent_req_call resulted in req being freed,
which freed state by extension and then the subsequent _post call was a
use-after-free. This patch saves the two variables used outside state so
that it's safe to use them even after the callback.
---
src/providers/ldap/sdap_async_groups.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index f52bbb6e3976117f647912ee73497568850e800d..1e9fb6dc4506594bf0a2172f266917ca53946efa 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -3224,14 +3224,20 @@ static errno_t sdap_nested_group_lookup_user(struct tevent_req *req,
return ret;
} else if (ret == EOK) {
DEBUG(SSSDBG_TRACE_FUNC, ("All done.\n"));
+
+ /* Calling tevent req done might invoke callback which would
+ * zero out state */
+ bool is_finished = state->send_finished;
+ struct tevent_context *ev = state->ev;
+
tevent_req_done(req);
/**
* FIXME: Rewrite nested group processing so we call
* tevent_req_post() only in _send().
*/
- if (state->send_finished == false) {
- tevent_req_post(req, state->ev);
+ if (is_finished == false) {
+ tevent_req_post(req, ev);
}
}
return EOK;
@@ -3288,14 +3294,20 @@ static errno_t sdap_nested_group_lookup_group(struct tevent_req *req)
return ret;
} else if (ret == EOK) {
DEBUG(SSSDBG_TRACE_FUNC, ("All done.\n"));
+
+ /* Calling tevent req done might invoke callback which would
+ * zero out state */
+ bool is_finished = state->send_finished;
+ struct tevent_context *ev = state->ev;
+
tevent_req_done(req);
/**
* FIXME: Rewrite nested group processing so we call
* tevent_req_post() only in _send().
*/
- if (state->send_finished == false) {
- tevent_req_post(req, state->ev);
+ if (is_finished == false) {
+ tevent_req_post(req, ev);
}
}
return EOK;
--
1.8.3.1
More information about the sssd-devel
mailing list