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

[raphael.ouazana@linagora.com]

Selon Pierangelo Masarati <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.

Ok.

> > 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)

Seems to be ok now.

> > 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.

It should maybe be called before testing if lc->conns[ i ].ld is not null.

> > 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.

Seems to be ok now.

> > 
> > 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.

Ok, your fix is simplier and works well :)

> 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.

I'll test it with heavy stress and I'll give you a feedback if I see
other leaks.

Raphaël Ouazana.