[SSSD] [PATCH] Process all groups from a single nesting level

Jakub Hrozek jhrozek at redhat.com
Thu Aug 16 22:25:38 UTC 2012


https://bugzilla.redhat.com/show_bug.cgi?id=846664

If the first group was cached when processing the nested group membership,
we would call tevent_req_done, effectivelly marking the whole nesting
level as done. That is wrong, we should only move to next group in the
same nesting level.

Attached are two patches. One that applies on the 1.5 branch and another
one that can be applied on top of both 1.8 and master.

I haven't yet figured out why we're not seeing the bug on master even
though the buggy code is there. Probably this issue is being shadowed b
another bug in the ghost users..I'll investigate more.

For now, I'm sending the patches for review to unblock the branches that
are affected by the bug.
-------------- next part --------------
>From 074ee693e7973b43444bb276db716014ba627a0c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 17 Aug 2012 00:09:48 +0200
Subject: [PATCH] Process all groups from a single nesting level

https://bugzilla.redhat.com/show_bug.cgi?id=846664

If the first group was cached when processing the nested group membership,
we would call tevent_req_done, effectivelly marking the whole nesting
level as done.
---
 src/providers/ldap/sdap_async_initgroups.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index b0b533242c83409d7ec1fd9ff721d7a59cb01556..36dacc16e9416deb6274b4ae145003dacc3ca755 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2010,9 +2010,8 @@ struct tevent_req *rfc2307bis_nested_groups_send(
     if ((num_groups == 0) ||
         (nesting > dp_opt_get_int(opts->basic, SDAP_NESTING_LEVEL))) {
         /* No parent groups to process or too deep*/
-        tevent_req_done(req);
-        tevent_req_post(req, ev);
-        return req;
+        ret = EOK;
+        goto done;
     }
 
     state->ev = ev;
@@ -2046,7 +2045,18 @@ struct tevent_req *rfc2307bis_nested_groups_send(
         goto done;
     }
 
-    ret = rfc2307bis_nested_groups_step(req);
+    while (state->group_iter < state->num_groups) {
+        ret = rfc2307bis_nested_groups_step(req);
+        if (ret == EOK) {
+            /* This group had already been looked up. Continue to
+             * another group in the same level
+             */
+            state->group_iter++;
+            continue;
+        } else {
+            goto done;
+        }
+    }
 
 done:
     if (ret == EOK) {
-- 
1.7.11.2

-------------- next part --------------
>From 0dbb40ea98958fc0a697c89939b1809bd207443b Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 16 Aug 2012 17:35:53 -0400
Subject: [PATCH] Process all groups from a single nesting level

https://bugzilla.redhat.com/show_bug.cgi?id=846664

If the first group was cached when processing the nested group membership,
we would call tevent_req_done, effectivelly marking the whole nesting
level as done.
---
 src/providers/ldap/sdap_async_accounts.c |   32 +++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index 39caf951873e551f5bb8f6da38c5656c1ad4f638..45817458db064d119f0a8bef77d00ea518b5bd2a 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -4693,9 +4693,8 @@ struct tevent_req *rfc2307bis_nested_groups_send(
     if ((num_groups == 0) ||
         (nesting > dp_opt_get_int(opts->basic, SDAP_NESTING_LEVEL))) {
         /* No parent groups to process or too deep*/
-        tevent_req_done(req);
-        tevent_req_post(req, ev);
-        return req;
+        ret = EOK;
+        goto immediate;
     }
 
     state->ev = ev;
@@ -4709,17 +4708,31 @@ struct tevent_req *rfc2307bis_nested_groups_send(
     state->nesting_level = nesting;
     state->group_hash = group_hash;
 
-    ret = rfc2307bis_nested_groups_step(req);
+    while (state->group_iter < state->num_groups) {
+        ret = rfc2307bis_nested_groups_step(req);
+        if (ret == EOK) {
+            /* This group had already been looked up. Continue to
+             * another group in the same level
+             */
+            state->group_iter++;
+            continue;
+        } else if (ret == EAGAIN) {
+            /* EAGAIN means a lookup is in progress */
+            return req;
+        } else {
+            goto immediate;
+        }
+    }
+
+immediate:
     if (ret == EOK) {
-        /* All parent groups were already processed */
+        /* All groups on that level had been processed. Quit. */
         tevent_req_done(req);
-        tevent_req_post(req, ev);
-    } else if (ret != EAGAIN) {
+    } else {
         tevent_req_error(req, ret);
-        tevent_req_post(req, ev);
     }
 
-    /* EAGAIN means a lookup is in progress */
+    tevent_req_post(req, ev);
     return req;
 }
 
@@ -4762,6 +4775,7 @@ static errno_t rfc2307bis_nested_groups_step(struct tevent_req *req)
     DEBUG(6, ("Processing group [%s]\n", state->primary_name));
 
     if (hash_has_key(state->group_hash, &key)) {
+        DEBUG(6, ("Group was already processed, taking a shortcut\n"));
         talloc_free(key.str);
         talloc_free(tmp_ctx);
         return EOK;
-- 
1.7.4.1



More information about the sssd-devel mailing list