suspected memory leaks in slapd (openldap-981116)

[Jorg Pietschmann <pietsch@swissline.ch>]

Hi,

there may be a few memory leaks in servers/slapd/add.c (do_add),
at least one of them serious (bloody f**** tabs replaced for space
conservation):

do_add( Connection *conn, Operation *op )
{
[snip decls and comments]
  e = (Entry *) ch_calloc( 1, sizeof(Entry) );
  /* initialize reader/writer lock */
  entry_rdwr_init(e);

  /* get the name */
  if ( ber_scanf( ber, "{a", &dn ) == LBER_ERROR ) {
    Debug( LDAP_DEBUG_ANY, "ber_scanf failed\n", 0, 0, 0 );
    send_ldap_result( conn, op, LDAP_PROTOCOL_ERROR, NULL,
        "decoding error" );
--- if you bail out here, the entry E will never be freed
    return;
  }
--- perhaps E should be allocated here (but see note before invocation
--- of be->be_add).

  e->e_dn = dn;
  dn = dn_normalize( strdup( dn ) );
--- Apparently DN is now allocated on the heap, but it is never freed
--- (unless this is hidden in select_backend, but still...).
--- BTW: why does the entry get the raw dn? Are you sure the statements
--- above are in the intended order?
  Debug( LDAP_DEBUG_ARGS, "    do_add: dn (%s)\n", dn, 0, 0 );
  /* get the attrs */
  e->e_attrs = NULL;
  for ( tag = ber_first_element( ber, &len, &last ); tag !=
LBER_DEFAULT;
    tag = ber_next_element( ber, &len, last ) ) {
    char		*type;
    struct berval	**vals;

    if ( ber_scanf( ber, "{a{V}}", &type, &vals ) == LBER_ERROR ) {
      send_ldap_result( conn, op, LDAP_PROTOCOL_ERROR,
	    NULL, "decoding error" );
--- TYPE and VALS may be allocated on the heap, and should be freed,
--- unless ber_scanf is intelligent enough to do this (i don't
--- think it is)
      entry_free( e );
      return;
    }

    if ( vals == NULL ) {
	Debug( LDAP_DEBUG_ANY, "no values for type %s\n", type,
	    0, 0 );
	send_ldap_result( conn, op, LDAP_PROTOCOL_ERROR, NULL,
	    NULL );
--- TYPE is definitely allocated here and should be freed.
	entry_free( e );
	return;
    }

    attr_merge( e, type, vals );

    free( type );
    ber_bvecfree( vals );
  }
[some stuff snipped]
  if ( be->be_add != NULL ) {
    /* do the update here */
    if ( be->be_updatedn == NULL ||
	strcasecmp( be->be_updatedn, op->o_dn ) == 0 ) {
--- Actually, allocating E and parsing of the attributes could be 
--- deferred until this point, i think.
      if ( (be->be_lastmod == ON || (be->be_lastmod == UNDEFINED &&
		global_lastmod == ON)) && be->be_updatedn == NULL ) {
	add_created_attrs( op, e );
      }
      if ( (*be->be_add)( be, conn, op, e ) == 0 ) {
	replog( be, LDAP_REQ_ADD, e->e_dn, e, 0 );
      }
[rest snipped]
}


Well, switch to Java for the added benefit of a garbage collector...

Another nitpick: dn_normalize wont mess up the (invalid) dn
`foo bar=sample' but silently truncates `foo="bar" sample'. This
is somewhat inconsistent.

J.Pietschmann