On 05/06/2010 02:45 PM, Stephen Gallagher wrote:
On 05/06/2010 02:32 PM, Stephen Gallagher wrote:
On 05/06/2010 02:11 PM, Dmitri Pal wrote:
Stephen Gallagher wrote:
ctx->gsh is sometimes NULL here, so dereferencing it without checking
is bad.
------------------------------------------------------------------------
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Through I do not have a problem with this specific patch I do have
several concerns about the underlaying code.
1) The code seems to be duplicated multiple times. Why common code is
not segmented into a common function?
It is now :)
2) What happens if the ctx->gsh is actually null? Is it safe to fall
through. May be a comment would help.
Comment added.
3) The code around sdap_account_info_restart(breq); is not consistent.
In first two cases we return in case of success. What do we do in case
of failure?
In the last function we on the contrary do not check the error and
return right away regardless of the outcome of the
sdap_account_info_restart(breq). This does not look right.
The third case was wrong. Good catch! I have now fixed this.
New patch attached. Thanks for the review.
Sorry, accidentally attached the old patch. This should be the right one
now.
Wow, I really need sleep.
Maybe I'll get it right this time?
--
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
From ad7b48b3010b6eb0ce55aa7d3484bb630de8b4ac Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Thu, 6 May 2010 11:23:23 -0400
Subject: [PATCH 1/2] Fix segfault in GSSAPI reconnect code
---
src/providers/ldap/ldap_id.c | 28 ++++++++++++++++++++--------
1 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index c472d8b6439514b027b4627f397777933c79db01..cf069c6c2b2e5a43d85848ee18baee725e8604c0 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -724,6 +724,21 @@ static int sdap_account_info_restart(struct be_req *breq)
return EOK;
}
+int sdap_start_over (struct sdap_id_ctx *ctx, struct be_req *breq)
+{
+ if (ctx->gsh) {
+ /* Mark the connection as false so we don't try to use an
+ * invalid connection by mistake later.
+ * If the global sdap handler is NULL, it's ok not to do
+ * anything here. It's always checked by sdap_connected()
+ * before being used.
+ */
+ ctx->gsh->connected = false;
+ }
+
+ return sdap_account_info_restart(breq);
+}
+
static void sdap_account_info_users_done(struct tevent_req *req)
{
struct be_req *breq = tevent_req_callback_data(req, struct be_req);
@@ -744,8 +759,7 @@ static void sdap_account_info_users_done(struct tevent_req *req)
ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
struct sdap_id_ctx);
if (sdap_check_gssapi_reconnect(ctx)) {
- ctx->gsh->connected = false;
- err = sdap_account_info_restart(breq);
+ err = sdap_start_over(ctx, breq);
if (err == EOK) return;
}
sdap_mark_offline(ctx);
@@ -775,8 +789,7 @@ static void sdap_account_info_groups_done(struct tevent_req *req)
ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
struct sdap_id_ctx);
if (sdap_check_gssapi_reconnect(ctx)) {
- ctx->gsh->connected = false;
- err = sdap_account_info_restart(breq);
+ err = sdap_start_over(ctx, breq);
if (err == EOK) return;
}
sdap_mark_offline(ctx);
@@ -792,7 +805,7 @@ static void sdap_account_info_initgr_done(struct tevent_req *req)
struct sdap_id_ctx *ctx;
int dp_err = DP_ERR_OK;
const char *error = NULL;
- int ret;
+ int ret, err;
ret = groups_by_user_recv(req);
talloc_zfree(req);
@@ -806,9 +819,8 @@ static void sdap_account_info_initgr_done(struct tevent_req *req)
ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
struct sdap_id_ctx);
if (sdap_check_gssapi_reconnect(ctx)) {
- ctx->gsh->connected = false;
- sdap_account_info_restart(breq);
- return;
+ err = sdap_start_over(ctx, breq);
+ if (err == EOK) return;
}
sdap_mark_offline(ctx);
}
--
1.7.0.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
