[SSSD] [PATCH] Two deref fixes

Jakub Hrozek jhrozek at redhat.com
Tue Nov 1 15:26:01 UTC 2011


On Tue, Nov 01, 2011 at 04:13:20PM +0100, Jan Zelený wrote:
> > Simo was studying the dereference code lately and found two issues. #1
> > is an important bug, #2 is more of a cosmetic issue.
> > 
> > [PATCH 1/2] Use LDAPDerefSpec properly
> > ldap_create_deref_control_value expects an array of LDAPDerefSpec
> > structures with LDAPDerefSpec.derefAttr == NULL as a sentinel. We were
> > passing a single instance of a LDAPDerefSpec structure.
> > 
> > https://fedorahosted.org/sssd/ticket/1050
> > 
> > 
> > [PATCH 2/2] Remove confusing do-while loop
> > The deref processing would return a single control back. The do-while
> > loop was harmless but confusing.
> 
> Nack,
> as we discussed earlier, please use new debug levels in the second patch.
> 
> Jan

Done.
-------------- next part --------------
From db45393eb861eacea9b04828999d15cdc873a0c4 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 21 Oct 2011 09:04:35 +0200
Subject: [PATCH 1/2] Use LDAPDerefSpec properly

ldap_create_deref_control_value expects an array of LDAPDerefSpec structures
with LDAPDerefSpec.derefAttr == NULL as a sentinel. We were passing a
single instance of a LDAPDerefSpec structure.

https://fedorahosted.org/sssd/ticket/1050
---
 src/providers/ldap/sdap_async.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 5d7019988fc3ec373179c6956abfa0e32b56a620..33664f146ec97e7b245bc8289c32634a5b9ce578 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1371,12 +1371,14 @@ static int sdap_x_deref_create_control(struct sdap_handle *sh,
 {
     struct berval derefval;
     int ret;
-    static LDAPDerefSpec ds;
+    struct LDAPDerefSpec ds[2];
 
-    ds.derefAttr = discard_const(deref_attr);
-    ds.attributes = discard_const(attrs);
+    ds[0].derefAttr = discard_const(deref_attr);
+    ds[0].attributes = discard_const(attrs);
 
-    ret = ldap_create_deref_control_value(sh->ldap, &ds, &derefval);
+    ds[1].derefAttr = NULL; /* sentinel */
+
+    ret = ldap_create_deref_control_value(sh->ldap, ds, &derefval);
     if (ret != LDAP_SUCCESS) {
         DEBUG(1, ("sss_ldap_control_create failed: %s\n",
                   ldap_err2string(ret)));
-- 
1.7.6.4

-------------- next part --------------
From 3efdbc0a25e418cc9d0edf91f7b253553ddaaf04 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 21 Oct 2011 09:23:36 +0200
Subject: [PATCH 2/2] Remove confusing do-while loop

The deref processing would return a single control back. The do-while
loop was harmless but confusing.
---
 src/providers/ldap/sdap_async.c |   71 ++++++++++++++++++++-------------------
 1 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 33664f146ec97e7b245bc8289c32634a5b9ce578..384ff8ef6a7612c81e0c5edf9814ecad4f3dfa30 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1402,8 +1402,6 @@ static errno_t sdap_x_deref_parse_entry(struct sdap_handle *sh,
 {
     errno_t ret;
     LDAPControl **ctrls = NULL;
-    LDAPControl **next = NULL;
-    LDAPControl **start = NULL;
     LDAPControl *derefctrl = NULL;
     LDAPDerefRes    *deref_res = NULL;
     LDAPDerefRes    *dref;
@@ -1419,55 +1417,58 @@ static errno_t sdap_x_deref_parse_entry(struct sdap_handle *sh,
     ret = ldap_get_entry_controls(state->sh->ldap, msg->msg,
                                   &ctrls);
     if (ret != LDAP_SUCCESS) {
-        DEBUG(1, ("ldap_parse_result failed\n"));
+        DEBUG(SSSDBG_OP_FAILURE, ("ldap_parse_result failed\n"));
         goto done;
     }
 
     if (!ctrls) {
-        DEBUG(4, ("No controls found for entry\n"));
+        DEBUG(SSSDBG_MINOR_FAILURE, ("No controls found for entry\n"));
         ret = ENOENT;
         goto done;
     }
 
-    start = ctrls;
     res = NULL;
-    do {
-        derefctrl = ldap_control_find(LDAP_CONTROL_X_DEREF, start, &next);
-        if (!derefctrl) break;
 
-        DEBUG(9, ("Got deref control\n"));
-        start = next;
+    derefctrl = ldap_control_find(LDAP_CONTROL_X_DEREF, ctrls, NULL);
+    if (!derefctrl) {
+        DEBUG(SSSDBG_FUNC_DATA, ("No deref controls found\n"));
+        ret = EOK;
+        goto done;
+    }
 
-        ret = ldap_parse_derefresponse_control(state->sh->ldap,
-                                               derefctrl,
-                                               &deref_res);
-        if (ret != LDAP_SUCCESS) {
-            DEBUG(1, ("ldap_parse_derefresponse_control failed: %s\n",
-                      ldap_err2string(ret)));
+    DEBUG(SSSDBG_TRACE_FUNC, ("Got deref control\n"));
+
+    ret = ldap_parse_derefresponse_control(state->sh->ldap,
+                                            derefctrl,
+                                            &deref_res);
+    if (ret != LDAP_SUCCESS) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("ldap_parse_derefresponse_control failed: %s\n",
+               ldap_err2string(ret)));
+        goto done;
+    }
+
+    for (dref = deref_res; dref; dref=dref->next) {
+        ret = sdap_parse_deref(tmp_ctx, state->maps, state->num_maps,
+                                state->sh, dref, &res);
+        if (ret) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sdap_parse_deref failed [%d]: %s\n",
+                  ret, strerror(ret)));
             goto done;
         }
 
-        for (dref = deref_res; dref; dref=dref->next) {
-            ret = sdap_parse_deref(tmp_ctx, state->maps, state->num_maps,
-                                   state->sh, dref, &res);
-            if (ret) {
-                DEBUG(1, ("sdap_parse_deref failed [%d]: %s\n",
-                          ret, strerror(ret)));
-                goto done;
-            }
-
-            ret = add_to_deref_reply(state, state->num_maps,
-                                     &state->dreply, res);
-            if (ret != EOK) {
-                DEBUG(1, ("add_to_deref_reply failed.\n"));
-                goto done;
-            }
+        ret = add_to_deref_reply(state, state->num_maps,
+                                    &state->dreply, res);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("add_to_deref_reply failed.\n"));
+            goto done;
         }
+    }
 
-        DEBUG(9, ("All deref results from a single control parsed\n"));
-        ldap_derefresponse_free(deref_res);
-        deref_res = NULL;
-    } while (derefctrl);
+    DEBUG(SSSDBG_TRACE_FUNC,
+          ("All deref results from a single control parsed\n"));
+    ldap_derefresponse_free(deref_res);
+    deref_res = NULL;
 
     ret = EOK;
 done:
-- 
1.7.6.4



More information about the sssd-devel mailing list