complex searches with filters: more memory leaks and an id list issue

[Thomas Koch <Thomas.Koch@atlantec-es.com>]

Hi,

as reported some time ago, our use of slapd involves all sorts of
search queries. We are now moving to 2.0.15 and believe that some
problems that we identified in 2.0.11 during the last weeks still
exist.

Included is a patch of the changes that we had applied to 2.0.11 recently
(with the intention to report them soon) and which we believe 
should go into 2.0.15.

* memory leaks: there are some locations in filterindex.c which
  do not free a tmp object under certain error conditions.

* null pointer: in filter.c, we have observed crashes when 
  an attempt was made to free f->f_sub, with f_sub being NULL.

* id list scan: in idl.c. This seems the most tricky issue. Our
  "fix" might be totally inconsistent, since it is time consuming
  to figure out all the details of the id list handling.
  Here is what we have observed: when matching id lists,
  it could happen that the same id occurs twice in the result.
  This could cause the resulting id list to grow beyond its
  allocated size, which then may result in a memory overrun.
  Our change enforces a more defensive behaviour, and also
  avoids inserting duplicates in the result id list.
  (In our test code there is an additional check against
   buffer overrun, which should probably not be used in production.)
  Whether this completely resolves the problem, I don't know,
  but we have not seen any of the problems after this change.

I would appreciate any feedback, because we don't want to
mess up the original code.

Regards,

Thomas


*** ./servers/slapd/back-ldbm/idl.c-dist	Sun Jul 22 04:28:29 2001
--- ./servers/slapd/back-ldbm/idl.c	Fri Oct  5 12:06:11 2001
***************
*** 882,891 ****
  			;	/* NULL */
  		}
! 
! 		if ( bi == ID_BLOCK_NIDS(b) ) {
  			break;
  		}
  
! 		if ( ID_BLOCK_ID(b, bi) == ID_BLOCK_ID(a, ai) ) {
  			ID_BLOCK_ID(n, ni++) = ID_BLOCK_ID(a, ai);
  		}
--- 882,892 ----
  			;	/* NULL */
  		}
! 		
! 		if ( bi >= ID_BLOCK_NIDS(b) ) {
  			break;
  		}
  
! 		if ( ( ID_BLOCK_ID(b, bi) == ID_BLOCK_ID(a, ai) )
! 		     && (ni == 0 || ID_BLOCK_ID(n, ni - 1) != ID_BLOCK_ID(b, bi)))  {
  			ID_BLOCK_ID(n, ni++) = ID_BLOCK_ID(a, ai);
  		}
*** ./servers/slapd/back-ldbm/filterindex.c-dist	Tue Sep  4 00:18:59 2001
--- ./servers/slapd/back-ldbm/filterindex.c	Fri Oct  5 12:12:07 2001
***************
*** 285,288 ****
--- 285,291 ----
  			idl_free( idl );
  			idl = NULL;
+ 			if ( tmp != NULL ) {
+ 			    idl_free( tmp );
+ 			}
  			Debug( LDAP_DEBUG_TRACE,
  				"<= equality_candidates key read failed (%d)\n",
***************
*** 412,419 ****
  
  		rc = key_read( be, db, keys[i], &tmp );
! 
  		if( rc != LDAP_SUCCESS ) {
  			idl_free( idl );
  			idl = NULL;
  			Debug( LDAP_DEBUG_TRACE, "<= approx_candidates key read failed (%d)\n",
  			    rc, 0, 0 );
--- 415,425 ----
  
  		rc = key_read( be, db, keys[i], &tmp );
! 		
  		if( rc != LDAP_SUCCESS ) {
  			idl_free( idl );
  			idl = NULL;
+ 			if ( tmp != NULL ) {
+ 			    idl_free( tmp );
+ 			}
  			Debug( LDAP_DEBUG_TRACE, "<= approx_candidates key read failed (%d)\n",
  			    rc, 0, 0 );
***************
*** 581,584 ****
--- 587,593 ----
  			idl_free( idl );
  			idl = NULL;
+ 			if ( tmp != NULL ) {
+ 			    idl_free( tmp );
+ 			}
  			Debug( LDAP_DEBUG_TRACE, "<= substrings_candidates key read failed (%d)\n",
  			    rc, 0, 0 );
*** ./servers/slapd/filter.c-dist	Fri Aug 31 22:24:17 2001
--- ./servers/slapd/filter.c	Fri Oct  5 11:45:21 2001
***************
*** 573,577 ****
  			ber_bvfree( f->f_sub_final );
  		}
! 		ch_free( f->f_sub );
  		break;
  
--- 573,579 ----
  			ber_bvfree( f->f_sub_final );
  		}
! 		if (f->f_sub != NULL) {
! 		    ch_free( f->f_sub );
! 		}
  		break;