On Thu, Jun 26, 2014 at 01:39:24PM +0200, Lukas Slebodnik wrote:
On (26/06/14 13:00), Sumit Bose wrote:
>Hi,
>
>I found this while debugging timeout issues during authentication.
>
>bye,
>Sumit
>From 03879293f434cf2aaa6db9ead75830ba4a7050a7 Mon Sep 17 00:00:00 2001
>From: Sumit Bose <sbose(a)redhat.com>
>Date: Thu, 26 Jun 2014 11:59:55 +0200
>Subject: [PATCH] LDAP: sdap_auth_send() properly initialize ppolicy
>
>In case of an error ppolicy might not be set to NULL and
>auth_bind_user_done() might show some unexpected debug messages.
>---
> src/providers/ldap/sdap_async_connection.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/src/providers/ldap/sdap_async_connection.c
b/src/providers/ldap/sdap_async_connection.c
>index a1f78c0..7407ae7 100644
>--- a/src/providers/ldap/sdap_async_connection.c
>+++ b/src/providers/ldap/sdap_async_connection.c
>@@ -1282,6 +1282,8 @@ struct tevent_req *sdap_auth_send(TALLOC_CTX *memctx,
> req = tevent_req_create(memctx, &state, struct sdap_auth_state);
> if (!req) return NULL;
>
>+ state->ppolicy = NULL;
>+
> if (sasl_mech) {
> state->is_sasl = true;
> subreq = sasl_bind_send(state, ev, sh, sasl_mech, sasl_user, NULL);
This would solve problem.
In my opinion, it would be better to solve in in function auth_bind_user_done
src/providers/ldap/ldap_auth.c:
errno_t sdap_auth_recv(struct tevent_req *req,
//snip
826 ret = sdap_auth_recv(subreq, state, &ppolicy)
827 talloc_zfree(subreq);
828 if (ppolicy != NULL) {
variable ret should be tested instead of ppolicy
829 DEBUG(SSSDBG_TRACE_ALL,"Found ppolicy data, "
BTW function sdap_auth_recv looks supspicious as well.
src/providers/ldap/sdap_async_connection.c:
1366 errno_t sdap_auth_recv(struct tevent_req *req,
1367 TALLOC_CTX *memctx,
1368 struct sdap_ppolicy_data **ppolicy)
1369 {
1370 struct sdap_auth_state *state = tevent_req_data(req,
1371 struct sdap_auth_state);
1372
1373 if (ppolicy != NULL) {
1374 *ppolicy = talloc_steal(memctx, state->ppolicy);
1375 }
1376
1377 TEVENT_REQ_RETURN_ON_ERROR(req);
this macro used to be called before talloc_steal in other recv
functions.
1378
1379 return EOK;
1380 }
Thank you, these are good points. Nevertheless I think it is a good idea
and common practice to initialize the members of a state ...
It is possible I overlooked something.
... to make other usage not fail.
New version attached.
bye,
Sumit
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel