Re: (ITS#5853) LDAP search crashes when chasing multiple referrals

[jsafrane@redhat.com]

This is a multi-part message in MIME format.
--------------090400040509090303060604
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

ando@sys-net.it wrote:
> jsafrane@redhat.com wrote:
> 
>> Following ldapsearch crashes when referral chasing is enabled in very unusual
>> environment with many AD servers referring to each other (I had to increase
>> refhoplimit to chase all the referrals).
> 
> Thanks for the detailed report.  However, IMHO, automatic referral 
> chasing Is Broken.  Clients should explicitly chase them, possibly 
> providing some interactive means for rebinding.  This is what is 
> currently done, for example, by slapd when using slapo-chain(5), and 
> what should be done by proxy backends as well.

nss_ldap does not have an option to chase referrals, it depends on ldap 
libraries to do the dirty job. I think that referral chasing in OpenLDAP 
libraries works pretty well apart my crash. Is there anything I am not 
aware of? Instead adding new features to nss_ldap, it would be better to 
fix OpenLDAP, more applications would benefit from that.

I tried to work around the crash, see patch attached (apply to current 
2.4 branch in CVS). I was not able to prevent deleting connections 
during referral chasing (I admit I got lost in the code and various code 
paths). So I added very simple mechanism to detect possibly freed 
connection and restart the loop in wait4msg(). It seems to help in my 
scenario.

Jan


--------------090400040509090303060604
Content-Type: text/plain;
 name="openldap-2.4-refer-crash.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="openldap-2.4-refer-crash.patch"

Index: libraries/libldap/ldap-int.h
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/libraries/libldap/ldap-int.h,v
retrieving revision 1.168.2.9
diff -u -r1.168.2.9 ldap-int.h
--- libraries/libldap/ldap-int.h	8 Nov 2008 00:14:45 -0000	1.168.2.9
+++ libraries/libldap/ldap-int.h	14 Jan 2009 14:54:09 -0000
@@ -401,6 +401,7 @@
 
 	LDAPConn	*ld_defconn;	/* default connection */
 	LDAPConn	*ld_conns;	/* list of server connections */
+	int		ld_conns_version;	/* unique id tracking deletions in ld_conns list */
 	void		*ld_selectinfo;	/* platform specifics for select */
 };
 #define LDAP_VALID(ld)		( (ld)->ld_valid == LDAP_VALID_SESSION )
Index: libraries/libldap/request.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/libraries/libldap/request.c,v
retrieving revision 1.125.2.12
diff -u -r1.125.2.12 request.c
--- libraries/libldap/request.c	8 Nov 2008 01:15:17 -0000	1.125.2.12
+++ libraries/libldap/request.c	14 Jan 2009 14:54:09 -0000
@@ -697,6 +698,7 @@
 			}
 			prevlc = tmplc;
 		}
+		ld->ld_conns_version++;
 #ifdef LDAP_R_COMPILE
 		ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
 #endif
Index: libraries/libldap/result.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/libraries/libldap/result.c,v
retrieving revision 1.124.2.14
diff -u -r1.124.2.14 result.c
--- libraries/libldap/result.c	10 Nov 2008 17:39:16 -0000	1.124.2.14
+++ libraries/libldap/result.c	14 Jan 2009 14:54:09 -0000
@@ -396,6 +396,7 @@
 						if ( lc->lconn_status == LDAP_CONNST_CONNECTED &&
 							ldap_is_read_ready( ld, lc->lconn_sb ) )
 						{
+							int version = ld->ld_conns_version;
 #ifdef LDAP_R_COMPILE
 							ldap_pvt_thread_mutex_unlock( &ld->ld_conn_mutex );
 #endif
@@ -403,12 +404,12 @@
 #ifdef LDAP_R_COMPILE
 							ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
 #endif
-							if ( lc == NULL ) {
+							if ( version != ld->ld_conns_version || lc == NULL ) {
 								/* if lc gets free()'d,
 								 * there's no guarantee
-								 * lc->lconn_next is still
+								 * lc or lc->lconn_next is still
 								 * sane; better restart
-								 * (ITS#4405) */
+								 * (ITS#4405, ITS#5853) */
 								lc = ld->ld_conns;
 
 								/* don't get to next conn! */

--------------090400040509090303060604--