[Date Prev][Date Next] [Chronological] [Thread] [Top]

RE: Assertion failure in try_read1msg() (ITS#2982)



If you look at the code for ldap_free_connection() you'll see that part of
this patch *shouldn't* be needed since it already walks thru ld->ld_conns and
unlinks the lc from the list. All that's needed is to assign nextlc in the
caller, to prevent referencing a freed conn struct. I'll apply a patch for
this...

  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support

> -----Original Message-----
> From: owner-openldap-bugs@OpenLDAP.org
> [mailto:owner-openldap-bugs@OpenLDAP.org]On Behalf Of lukeh@PADL.COM
> Sent: Friday, February 27, 2004 5:18 PM
> To: openldap-its@OpenLDAP.org
> Subject: RE: Assertion failure in try_read1msg() (ITS#2982)
>
>
>
> --19701020
> Content-Type: text/plain; charset=US-ASCII
> Content-Disposition: inline
>
>
> Sorry, try this patch.
>
> Turning referrals off in nss_ldap also appears to workaround the
> problem so perhaps I'll do that for now.
>
> -- Luke
>
>
> --19701020
> Content-Type: text/plain; name="result.c.diff"; x-unix-mode=0644
> Content-Disposition: attachment; filename="result.c.diff"
>
> --- result.c	2004-02-28 12:07:43.000000000 +1100
> +++ result.c.patch	2004-02-28 12:07:33.000000000 +1100
> @@ -73,7 +73,7 @@
>  static int wait4msg LDAP_P(( LDAP *ld, ber_int_t msgid, int
> all, struct timeval *timeout,
>  	LDAPMessage **result ));
>  static ber_tag_t try_read1msg LDAP_P(( LDAP *ld, ber_int_t msgid,
> -	int all, Sockbuf *sb, LDAPConn *lc, LDAPMessage **result ));
> +	int all, Sockbuf *sb, LDAPConn **lc, LDAPMessage **result ));
>  static ber_tag_t build_result_ber LDAP_P(( LDAP *ld,
> BerElement **bp, LDAPRequest *lr ));
>  static void merge_error_info LDAP_P(( LDAP *ld, LDAPRequest
> *parentr, LDAPRequest *lr ));
>  static LDAPMessage * chkResponseList LDAP_P(( LDAP *ld, int
> msgid, int all));
> @@ -261,7 +261,7 @@
>  	struct timeval	tv, *tvp;
>  	time_t		start_time = 0;
>  	time_t		tmp_time;
> -	LDAPConn	*lc, *nextlc;
> +	LDAPConn	**lc, **nextlc;
>
>  	assert( ld != NULL );
>  	assert( result != NULL );
> @@ -315,16 +315,20 @@
>              rc = (*result)->lm_msgtype;
>          } else {
>
> -			for ( lc = ld->ld_conns; lc != NULL; lc
> = lc->lconn_next ) {
> -				if ( ber_sockbuf_ctrl( lc->lconn_sb,
> +			for ( lc = &ld->ld_conns; *lc != NULL;
> lc = nextlc ) {
> +				nextlc = &((*lc)->lconn_next);
> +				if ( ber_sockbuf_ctrl( (*lc)->lconn_sb,
>
> LBER_SB_OPT_DATA_READY, NULL ) ) {
> -					    rc = try_read1msg(
> ld, msgid, all, lc->lconn_sb,
> +					    rc = try_read1msg(
> ld, msgid, all, (*lc)->lconn_sb,
>  					        lc, result );
> +					    if ( *lc == NULL ) {
> +						*lc = *nextlc;
> +					    }
>  				    break;
>  				}
>  	        }
>
> -		    if ( lc == NULL ) {
> +		    if ( *lc == NULL ) {
>  			    rc = ldap_int_select( ld, tvp );
>  #ifdef LDAP_DEBUG
>  			    if ( rc == -1 ) {
> @@ -365,15 +369,18 @@
>  #ifdef LDAP_R_COMPILE
>
> ldap_pvt_thread_mutex_unlock( &ld->ld_req_mutex );
>  #endif
> -				    for ( lc = ld->ld_conns; rc
> == -2 && lc != NULL;
> +				    for ( lc = &ld->ld_conns;
> rc == -2 && *lc != NULL;
>  				        lc = nextlc ) {
> -					    nextlc = lc->lconn_next;
> -					    if ( lc->lconn_status ==
> +					    nextlc =
> &((*lc)->lconn_next);
> +					    if ( (*lc)->lconn_status ==
>  					        LDAP_CONNST_CONNECTED &&
>  					        ldap_is_read_ready( ld,
> -					        lc->lconn_sb )) {
> +					        (*lc)->lconn_sb )) {
>  						    rc =
> try_read1msg( ld, msgid, all,
> -
> lc->lconn_sb, lc, result );
> +
> (*lc)->lconn_sb, lc, result );
> +						    if ( *lc == NULL ) {
> +							*lc = *nextlc;
> +						    }
>  					    }
>  				    }
>  			    }
> @@ -409,7 +416,7 @@
>  	ber_int_t msgid,
>  	int all,
>  	Sockbuf *sb,
> -	LDAPConn *lc,
> +	LDAPConn **lcp,
>  	LDAPMessage **result )
>  {
>  	BerElement	*ber;
> @@ -419,6 +426,7 @@
>  	ber_len_t	len;
>  	int		foundit = 0;
>  	LDAPRequest	*lr, *tmplr;
> +	LDAPConn	*lc;
>  	BerElement	tmpber;
>  	int		rc, refer_cnt, hadref, simple_request;
>  	ber_int_t	lderr;
> @@ -432,7 +440,8 @@
>  	int     v3ref;
>
>  	assert( ld != NULL );
> -	assert( lc != NULL );
> +	assert( lcp != NULL );
> +	assert( *lcp != NULL );
>
>  #ifdef NEW_LOGGING
>  	LDAP_LOG ( OPERATION, ARGS, "read1msg: msgid %d, all
> %d\n", msgid, all, 0 );
> @@ -440,6 +449,8 @@
>  	Debug( LDAP_DEBUG_TRACE, "read1msg: msgid %d, all
> %d\n", msgid, all, 0 );
>  #endif
>
> +	lc = *lcp;
> +
>  retry:
>  	if ( lc->lconn_ber == NULL ) {
>  		lc->lconn_ber = ldap_alloc_ber_with_options(ld);
> @@ -815,6 +826,7 @@
>
>  			if ( lc != NULL ) {
>  				ldap_free_connection( ld, lc, 0, 1 );
> +				*lcp = NULL;
>  			}
>  		}
>  	}
>
> --19701020
> Content-Type: text/plain; charset=US-ASCII
> Content-Disposition: inline
>
>
>
> --19701020--
>
>
>