On 05/07/2010 11:17 AM, Eugene Indenbom wrote:
Oops, sorry about stray comment. I forgot that in recent changes retry
count check has been added.
No problem. It's not obvious from that code right there.
Yes, when we reach retry limit we should go offline.
In case of out of memory (or other error) it seems more correct to
report an error.
But as out of memory is the only possible error here going offline is OK.
Yeah, OOM should be incredibly rare (and fairly obvious), so I'm not too
worried about that.
BTW Please take a look into src/providers/ldap_id_enum.c. It contains 2
inclusions of sdap_check_gssapi_reconnect that are almost identical to
what you are fixing now.
Thanks for noticing that. I didn't think to check the enumeration code.
I'm doing a simple fix for that section.
Hopefully this is the final patch now.
--
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 779ecc4696898af555d847360429ee417acbe91c Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Thu, 6 May 2010 11:23:23 -0400
Subject: [PATCH] Fix segfault in GSSAPI reconnect code
Also clean up some duplicated code into a single common routine
sdap_account_info_common_done()
---
src/providers/ldap/ldap_id.c | 110 +++++++++++++++----------------------
src/providers/ldap/ldap_id_enum.c | 8 ++-
2 files changed, 51 insertions(+), 67 deletions(-)
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index c472d8b6439514b027b4627f397777933c79db01..f0e96677868811c7ee7a6d1a20339dcf61a3d85f 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -724,96 +724,76 @@ static int sdap_account_info_restart(struct be_req *breq)
return EOK;
}
+static void sdap_account_info_common_done(int ret, struct be_req *breq,
+ const char *str_on_err)
+{
+ struct sdap_id_ctx *ctx;
+ int dp_err = DP_ERR_OK;
+ const char *errstr = NULL;
+ errno_t err;
+
+ if (ret != EOK) {
+ dp_err = DP_ERR_FATAL;
+ errstr = str_on_err;
+
+ if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
+ ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
+ struct sdap_id_ctx);
+ if (sdap_check_gssapi_reconnect(ctx)) {
+ 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;
+ }
+ err = sdap_account_info_restart(breq);
+ if (err == EOK) return;
+ }
+
+ /* Couldn't reconnect, that was our last try
+ * Go offline now
+ */
+ dp_err = DP_ERR_OFFLINE;
+ sdap_mark_offline(ctx);
+ }
+ }
+
+ sdap_handler_done(breq, dp_err, ret, errstr);
+}
+
static void sdap_account_info_users_done(struct tevent_req *req)
{
struct be_req *breq = tevent_req_callback_data(req, struct be_req);
- struct sdap_id_ctx *ctx;
- int dp_err = DP_ERR_OK;
- const char *error = NULL;
- int ret, err;
+ int ret;
ret = users_get_recv(req);
talloc_zfree(req);
- if (ret) {
- dp_err = DP_ERR_FATAL;
- error = "Enum Users Failed";
-
- if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
- dp_err = DP_ERR_OFFLINE;
- 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);
- if (err == EOK) return;
- }
- sdap_mark_offline(ctx);
- }
- }
-
- sdap_handler_done(breq, dp_err, ret, error);
+ sdap_account_info_common_done(ret, breq, "User lookup failed");
}
static void sdap_account_info_groups_done(struct tevent_req *req)
{
struct be_req *breq = tevent_req_callback_data(req, struct be_req);
- struct sdap_id_ctx *ctx;
- int dp_err = DP_ERR_OK;
- const char *error = NULL;
- int ret, err;
+ int ret;
ret = groups_get_recv(req);
talloc_zfree(req);
- if (ret) {
- dp_err = DP_ERR_FATAL;
- error = "Enum Groups Failed";
-
- if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
- dp_err = DP_ERR_OFFLINE;
- 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);
- if (err == EOK) return;
- }
- sdap_mark_offline(ctx);
- }
- }
-
- return sdap_handler_done(breq, dp_err, ret, error);
+ sdap_account_info_common_done(ret, breq, "Group lookup failed");
}
static void sdap_account_info_initgr_done(struct tevent_req *req)
{
struct be_req *breq = tevent_req_callback_data(req, struct be_req);
- struct sdap_id_ctx *ctx;
- int dp_err = DP_ERR_OK;
- const char *error = NULL;
int ret;
ret = groups_by_user_recv(req);
talloc_zfree(req);
- if (ret) {
- dp_err = DP_ERR_FATAL;
- error = "Init Groups Failed";
-
- if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
- dp_err = DP_ERR_OFFLINE;
- 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;
- }
- sdap_mark_offline(ctx);
- }
- }
-
- return sdap_handler_done(breq, dp_err, ret, error);
+ sdap_account_info_common_done(ret, breq, "Init Groups Failed");
}
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
index d86b38d0f1adb4a5e45122b84db8ff8be040e44a..89ca2de7afec7914d6bbfcefd2c430e971ad3518 100644
--- a/src/providers/ldap/ldap_id_enum.c
+++ b/src/providers/ldap/ldap_id_enum.c
@@ -234,7 +234,9 @@ fail:
(int)err, strerror(err)));
if (sdap_check_gssapi_reconnect(state->ctx)) {
- state->ctx->gsh->connected = false;
+ if (state->ctx->gsh) {
+ state->ctx->gsh->connected = false;
+ }
ret = ldap_id_enum_users_restart(req);
if (ret == EOK) return;
}
@@ -282,7 +284,9 @@ static void ldap_id_enum_groups_done(struct tevent_req *subreq)
fail:
/* check if credentials are expired otherwise go offline on failures */
if (sdap_check_gssapi_reconnect(state->ctx)) {
- state->ctx->gsh->connected = false;
+ if (state->ctx->gsh) {
+ state->ctx->gsh->connected = false;
+ }
ret = ldap_id_enum_groups_restart(req);
if (ret == EOK) return;
}
--
1.7.0.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
