On 01/23/2013 03:51 PM, Pavel Březina wrote:
On 01/23/2013 03:46 PM, Pavel Březina wrote:
On 01/23/2013 03:21 PM, Jakub Hrozek wrote:
On Wed, Jan 23, 2013 at 02:56:59PM +0100, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/1783

The proper way to fix this would be to rewrite return codes of involved
functions, so that we call tevent_req_done() and tevent_req_post() in
_send(). However, rewriting return codes may be tricky and
given the circumstances (blocker for 6.4 and deadline), I chose to call
tevent_req_post() directly and open a new ticket for the return codes:

https://fedorahosted.org/sssd/ticket/1784

I'm not sure if this is the right approach if there already *is* a
callback, please check if this fix also works for scenarios where there
is both correct and incorrect DN.

Right. Good catch. Tevent code says that the callback would be
triggered twice. Tevent doesn't provider any function to determine
whether there is a callback set or not. Possible hack:

tevent_req_post() adds new immediate event, that fires the callback.

             tevent_req_done(req);
             tevent_req_set_callback(req, NULL, NULL);
             tevent_req_post(req, state->ev);

If a callback is set, it will be triggered with tevent_req_done() and
then removed. Immediate event will be processed but basically as noop.

If it is not set, the callback will be triggered in immediate event.

I'm taking back my words. Callback usually frees the request so this
would likely end up with crash.

I'm attaching new patch. It is not pretty but it is the least invasive way that crossed my mind.

From b6e876689fdeefdc1740e3bec7f97d6c1bf2b8d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Mon, 28 Jan 2013 10:56:56 +0100
Subject: [PATCH] nested groups: fix group lookup hangs if member dn is
 incorrect

https://fedorahosted.org/sssd/ticket/1783

When dn in member attribute is invalid (e.g. rdn instead of dn)
or it is outside of configured search bases, we might hit a situation
when tevent_req is marked as done before any callback could be
attached on it.
---
 src/providers/ldap/sdap_async_groups.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 96cc7c0c1e308e7a8f0b714c2e34e01a62d76779..76c077d83be4a0ca0a5da3cf35889dbcf0c7e87d 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -2528,6 +2528,11 @@ struct sdap_nested_group_ctx {
 
     bool enable_deref;
     struct sdap_deref_ctx *derefctx;
+
+    /**
+     * FIXME: Remove me!
+     */
+    bool send_finished;
 };
 
 static errno_t sdap_nested_group_process_deref_step(struct tevent_req *req);
@@ -2564,6 +2569,7 @@ static struct tevent_req *sdap_nested_group_process_send(
     state->sh = sh;
     state->enable_deref = enable_deref;
     state->nesting_level = nesting;
+    state->send_finished = false;
 
     /* If this is too many levels deep, just return success */
     if (nesting > dp_opt_get_int(opts->basic, SDAP_NESTING_LEVEL)) {
@@ -2672,6 +2678,7 @@ static struct tevent_req *sdap_nested_group_process_send(
         if (ret != EAGAIN) goto immediate;
     }
 
+    state->send_finished = true;
     return req;
 
 immediate:
@@ -2681,6 +2688,7 @@ immediate:
         tevent_req_error(req, ret);
     }
     tevent_req_post(req, ev);
+    state->send_finished = true;
     return req;
 }
 
@@ -3209,6 +3217,14 @@ static errno_t sdap_nested_group_lookup_user(struct tevent_req *req,
             } else if (ret == EOK) {
                 DEBUG(SSSDBG_TRACE_FUNC, ("All done.\n"));
                 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);
+                }
             }
             return EOK;
         }
@@ -3265,6 +3281,14 @@ static errno_t sdap_nested_group_lookup_group(struct tevent_req *req)
         } else if (ret == EOK) {
             DEBUG(SSSDBG_TRACE_FUNC, ("All done.\n"));
             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);
+            }
         }
         return EOK;
     }
-- 
1.7.11.7

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to