Attachment 579913 Details for Bug 815846 – Preliminary version of bind-dyndb-ldap upstream patch to correct this issue

[patch] Preliminary version of bind-dyndb-ldap upstream patch to correct this issue

BZ#815846_preliminary_upstream_patch.txt (text/plain), 5.78 KB, created by Jan Lieskovsky on 2012-04-24 16:32:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Lieskovsky

Created: 2012-04-24 16:32:06 UTC

Size: 5.78 KB

patch

obsolete

>From 5629c7911fd2028eea1bc97d78bfec11b91920cb Mon Sep 17 00:00:00 2001
>From: Adam Tkac <atkac@redhat.com>
>Date: Mon, 23 Apr 2012 14:56:47 +0200
>Subject: [PATCH 1/2] Attempt to reconnect only once per query to avoid
> infinite loop and deadlock.
>
>Signed-off-by: Adam Tkac <atkac@redhat.com>
>---
> src/ldap_helper.c |   78 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 40 insertions(+), 38 deletions(-)
>
>diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>index 47c0559..a7c2d4d 100644
>--- a/src/ldap_helper.c
>+++ b/src/ldap_helper.c
>@@ -267,9 +267,8 @@ static isc_result_t ldap_connect(ldap_instance_t *ldap_inst,
> 		ldap_connection_t *ldap_conn, isc_boolean_t force);
> static isc_result_t ldap_reconnect(ldap_instance_t *ldap_inst,
> 		ldap_connection_t *ldap_conn, isc_boolean_t force);
>-static int handle_connection_error(ldap_instance_t *ldap_inst,
>-		ldap_connection_t *ldap_conn, isc_boolean_t force,
>-		isc_result_t *result);
>+static isc_result_t handle_connection_error(ldap_instance_t *ldap_inst,
>+		ldap_connection_t *ldap_conn, isc_boolean_t force);
> static isc_result_t ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> 		const char *base,
> 		int scope, char **attrs, int attrsonly, const char *filter, ...);
>@@ -1605,6 +1604,8 @@ ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> 	va_list ap;
> 	isc_result_t result;
> 	int cnt;
>+	int ret;
>+	int once = 0;
> 
> 	REQUIRE(ldap_conn != NULL);
> 
>@@ -1625,30 +1626,36 @@ ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> 			return result;
> 	}
> 
>-	do {
>-		int ret;
>+retry:
>+	ret = ldap_search_ext_s(ldap_conn->handle, base, scope,
>+				str_buf(ldap_conn->query_string),
>+				attrs, attrsonly, NULL, NULL, NULL,
>+				LDAP_NO_LIMIT, &ldap_conn->result);
>+	if (ret == 0) {
>+		ldap_conn->tries = 0;
>+		cnt = ldap_count_entries(ldap_conn->handle, ldap_conn->result);
>+		log_debug(2, "entry count: %d", cnt);
> 
>-		ret = ldap_search_ext_s(ldap_conn->handle, base, scope,
>-					str_buf(ldap_conn->query_string),
>-					attrs, attrsonly, NULL, NULL, NULL,
>-					LDAP_NO_LIMIT, &ldap_conn->result);
>-		if (ret == 0) {
>-			ldap_conn->tries = 0;
>-			cnt = ldap_count_entries(ldap_conn->handle, ldap_conn->result);
>-			log_debug(2, "entry count: %d", cnt);
>+		result = ldap_entrylist_create(ldap_conn->mctx,
>+					       ldap_conn->handle,
>+					       ldap_conn->result,
>+					       &ldap_conn->ldap_entries);
>+		if (result != ISC_R_SUCCESS) {
>+			log_error("failed to save LDAP query results");
>+			return result;
>+		}
> 
>-			result = ldap_entrylist_create(ldap_conn->mctx,
>-						       ldap_conn->handle,
>-						       ldap_conn->result,
>-						       &ldap_conn->ldap_entries);
>-			if (result != ISC_R_SUCCESS) {
>-				log_error("failed to save LDAP query results");
>-				return result;
>-			}
>+		return ISC_R_SUCCESS;
>+	}
> 
>-			return ISC_R_SUCCESS;
>-		}
>-	} while (handle_connection_error(ldap_inst, ldap_conn, ISC_FALSE, &result));
>+	/* some error happened during ldap_search, try to recover */
>+	if (!once) {
>+		once++;
>+		result = handle_connection_error(ldap_inst, ldap_conn,
>+						 ISC_FALSE);
>+		if (result == ISC_R_SUCCESS)
>+			goto retry;
>+	}
> 
> 	return result;
> }
>@@ -1901,15 +1908,13 @@ force_reconnect:
> 	return ISC_R_SUCCESS;
> }
> 
>-static int
>+static isc_result_t
> handle_connection_error(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
>-			isc_boolean_t force, isc_result_t *result)
>+			isc_boolean_t force)
> {
> 	int ret;
> 	int err_code;
> 
>-	*result = ISC_R_FAILURE;
>-
> 	ret = ldap_get_option(ldap_conn->handle, LDAP_OPT_RESULT_CODE,
> 			      (void *)&err_code);
> 
>@@ -1920,9 +1925,8 @@ handle_connection_error(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn
> 
> 	switch (err_code) {
> 	case LDAP_NO_SUCH_OBJECT:
>-		*result = ISC_R_SUCCESS;
> 		ldap_conn->tries = 0;
>-		return 0;
>+		return ISC_R_SUCCESS;
> 	case LDAP_TIMEOUT:
> 		log_error("LDAP query timed out. Try to adjust \"timeout\" parameter");
> 		break;
>@@ -1932,11 +1936,11 @@ handle_connection_error(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn
> reconnect:
> 		if (ldap_conn->tries == 0)
> 			log_error("connection to the LDAP server was lost");
>-		if (ldap_connect(ldap_inst, ldap_conn, force) == ISC_R_SUCCESS)
>-			return 1;
>+		return ldap_connect(ldap_inst, ldap_conn, force);
>+		
> 	}
> 
>-	return 0;
>+	return ISC_R_FAILURE;
> }
> 
> /* FIXME: Handle the case where the LDAP handle is NULL -> try to reconnect. */
>@@ -3121,16 +3125,14 @@ restart:
> 				  &conn->result);
> 
> 		if (ret <= 0) {
>-			int ok;
>-			while (!(ok = handle_connection_error(inst, conn, ISC_TRUE,
>-							&result))) {
>+			while (handle_connection_error(inst, conn, ISC_TRUE)
>+			       != ISC_R_SUCCESS) {
> 				log_error("ldap_psearch_watcher failed to handle "
> 					  "LDAP connection error. Reconnection "
> 					  "in %ds", inst->reconnect_interval);
> 				sleep(inst->reconnect_interval);
> 			}
>-			if (ok)
>-				goto restart;
>+			goto restart;
> 		}
> 
> 		switch (ret) {
>-- 
>1.7.10
>
>
>From 54fd2f2d293f0831a13e7ac8e963c3940301cec1 Mon Sep 17 00:00:00 2001
>From: Adam Tkac <atkac@redhat.com>
>Date: Mon, 23 Apr 2012 16:08:05 +0200
>Subject: [PATCH 2/2] Log invalid syntax error in handle_connection_error as a
> bug
>
>Signed-off-by: Adam Tkac <atkac@redhat.com>
>---
> src/ldap_helper.c |    4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>index a7c2d4d..6ebe4c0 100644
>--- a/src/ldap_helper.c
>+++ b/src/ldap_helper.c
>@@ -1930,6 +1930,10 @@ handle_connection_error(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn
> 	case LDAP_TIMEOUT:
> 		log_error("LDAP query timed out. Try to adjust \"timeout\" parameter");
> 		break;
>+	case LDAP_INVALID_DN_SYNTAX:
>+	case LDAP_INVALID_SYNTAX:
>+		log_bug("Invalid syntax in handle_connection_error indicates a bug");
>+		break;
> 	default:
> 		/* Try to reconnect on other errors. */
> 		log_error("LDAP error: %s", ldap_err2string(err_code));
>-- 
>1.7.10

Actions: View | Diff

Attachments on bug 815846: 579913