Re: Memory problems in back-meta (ITS#2986)

[ando@sys-net.it]

rouazana@linagora.com wrote:
> Full_Name: Raphael Ouazana
> Version: 2.2.5
> OS: Linux
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (194.98.7.155)
> 
> 
> 
> With Valgrind I've found some memory problems in back-meta :
> 
> 1. Memory leak with rebind-as-user
> 
> In bind.c:243 there is a memory allocation (ber_dupbv) witch is never
> desallocated.

got it.

> 
> 2. Memory leak in search.c
> 
> In search.c:367 res is allocated but not ever desallocated (I've still not
> exactly determined when it is and when it isn't).
> 

got it (exploited only in case of failure or referral)

> 3. Memory leak in conn.c
> 
> In conn.c:237 rewrite_session_init allocates memory witch is not desallocated.

should be handled in unbind.c, where rewrite_session_delete()
is called.

> 
> 4. Memory leaks in meta_back_op_result
> 
> meta_back_op_result is called by some operations. For exemple when it is called
> by meta_back_modify (modify.c:151) some pointers (msg and match) are not
> desallocated (they were allocated with ldap_get_option in bind.c:428 and
> bind.c:430).

got it.  Note that this function definitely needs some
improvements, e.g., as noted in comments, errors in different
candidates should be prioritized.

> 
> 5. Possible segfault in conn.c
> 
> In conn.c:251 ldap_back_dn_massage can give the same adress to lsc->bound_dn and
> op->o_conn->c_dn.
> But in slapd/connection.c connection_destroy will call
> backend_connection_destroy and then connection2anonymous.
> backend_connection_destroy will free lsc->bound_dn and connection2anonymous will
> free op->o_conn->c_dn, so this will cause a segfault.
> I think the following patch will prevent it without introducing any memory
> leak:
> 
> $ diff -u servers/slapd/back-meta/conn.c.orig servers/slapd/back-meta/conn.c
> --- servers/slapd/back-meta/conn.c.orig 2004-03-01 16:21:25.000000000 +0100
> +++ servers/slapd/back-meta/conn.c      2004-03-01 16:25:20.000000000 +0100
> @@ -240,6 +240,7 @@
>          * If the connection dn is not null, an attempt to rewrite it is made
>          */
>         if ( op->o_conn->c_dn.bv_len != 0 ) {
> +               struct berval tmp;
>                 dc.rwmap = &lt->rwmap;
>                 dc.conn = op->o_conn;
>                 dc.rs = rs;
> @@ -254,6 +255,17 @@
>                         return rs->sr_err;
>                 }
>   
> +               if ( lsc->bound_dn.bv_val && lsc->bound_dn.bv_val !=
> op->o_conn->c_dn.bv_val ) {
> +                       ber_memfree( lsc->bound_dn.bv_val );
> +                       lsc->bound_dn.bv_val = NULL;
> +                       lsc->bound_dn.bv_len = 0;
> +               }
> +
> +               ber_dupbv( &tmp, &lsc->bound_dn );
> +               ber_dupbv( &lsc->bound_dn, &tmp );
> +               if ( tmp.bv_val )
> +                       ber_memfree( tmp.bv_val );
> +
>                 assert( lsc->bound_dn.bv_val );
>   
>         } else {
> $
> 
> Raphael Ouazana.

Got it; your patch doesn't seem to fix the problem,
though.  Fixed differently.

Note that there might be some more leaks/memory problems;
if heavily stressed, slapd keeps growing (though at a low
pace).  I'll try to check it myself with valgrind.

p.


-- 
Dr. Pierangelo Masarati         mailto:pierangelo.masarati@sys-net.it
LDAP Architect, SysNet s.n.c.   http://www.sys-net.it



    SysNet - via Dossi,8 27100 Pavia Tel: +390382573859 Fax: +390382476497