Re: (ITS#4901) malloc/free mismatches in ldapadd
by quanah＠stanford.edu
--On Saturday, March 31, 2007 12:54 AM +0000 quanah(a)stanford.edu wrote:
> --On Friday, March 30, 2007 2:25 PM +0000 chris.ridd(a)isode.com wrote:
>
>> Full_Name: Chris Ridd
>> Version: 2.3.34
>> OS: Windows
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (62.3.217.250)
>>
>>
>> On Windows, it is important that mallocs done in one DLL are balanced by
>> frees done from the same DLL. Failure to do this consistently leads to
>> heap corruption.
>>
>> This was observed to occur in several places in ldapadd - several buffers
>> allocated by ber_memfree() were freed using free().
>>
>> Our customer has successfully used a version of ldapadd on Windows
>> containing this patch (and the one in ITS 4900.)
>
> Thanks, testing.
--- openldap-2.3.34/clients/tools/common.c.orig Fri Mar 30 15:17:32 2007
+++ openldap-2.3.34/clients/tools/common.c Fri Mar 30 15:21:42 2007
@@ -1244,7 +1244,7 @@
crit ? "critical " : "" );
}
- free( ctrls );
+ ldap_controls_free( ctrls );
if ( crit ) {
exit( EXIT_FAILURE );
}
Just to note, this part of the patch is bad. ctrls is a pointer to an
array of controls, not an array of controls itself. Therefore, the use of
"free" here is appropriate. Using ldap_controls_free results in segfaults.
--Quanah
--
Quanah Gibson-Mount
Senior Systems Software Developer
ITS/Shared Application Services
Stanford University
GnuPG Public Key: http://www.stanford.edu/~quanah/pgp.html
16 years, 8 months
Re: (ITS#4901) malloc/free mismatches in ldapadd
by quanah＠stanford.edu
--On Friday, March 30, 2007 2:25 PM +0000 chris.ridd(a)isode.com wrote:
> Full_Name: Chris Ridd
> Version: 2.3.34
> OS: Windows
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (62.3.217.250)
>
>
> On Windows, it is important that mallocs done in one DLL are balanced by
> frees done from the same DLL. Failure to do this consistently leads to
> heap corruption.
>
> This was observed to occur in several places in ldapadd - several buffers
> allocated by ber_memfree() were freed using free().
>
> Our customer has successfully used a version of ldapadd on Windows
> containing this patch (and the one in ITS 4900.)
Thanks, testing.
--Quanah
--
Quanah Gibson-Mount
Senior Systems Software Developer
ITS/Shared Application Services
Stanford University
GnuPG Public Key: http://www.stanford.edu/~quanah/pgp.html
16 years, 8 months
Re: (ITS#4900) command-line utils crash
by quanah＠stanford.edu
--On Friday, March 30, 2007 2:09 PM +0000 chris.ridd(a)isode.com wrote:
> Full_Name: Chris Ridd
> Version: 2.3.34
> OS: Windows
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (62.3.217.250)
>
>
> On Windows, the ldapadd tool often crashes while parsing the command-line
> arguments.
Hi Chris,
We formally dropped support for MSVC a long time ago. That being said,
working build of OpenLDAP was accomplished with MSVCC a few months ago
without hitting this issue. Which version of MSVC is being used, and
please doublecheck the conditions spelled out in the comments in
<ldap_cdefs.h>, where it starts with "Support for Windows DLLs".
Also, instead of declaring things as extern, does changing LDAP_LIBC_V to
LDAP_LUTIL_V resolve your issue?
--Quanah
--
Quanah Gibson-Mount
Senior Systems Software Developer
ITS/Shared Application Services
Stanford University
GnuPG Public Key: http://www.stanford.edu/~quanah/pgp.html
16 years, 8 months
(ITS#4901) malloc/free mismatches in ldapadd
by chris.ridd＠isode.com
Full_Name: Chris Ridd
Version: 2.3.34
OS: Windows
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (62.3.217.250)
On Windows, it is important that mallocs done in one DLL are balanced by frees
done from the same DLL. Failure to do this consistently leads to heap
corruption.
This was observed to occur in several places in ldapadd - several buffers
allocated by ber_memfree() were freed using free().
Our customer has successfully used a version of ldapadd on Windows containing
this patch (and the one in ITS 4900.)
Patch follows:
--- openldap-2.3.34/clients/tools/common.c.orig Fri Mar 30 15:17:32 2007
+++ openldap-2.3.34/clients/tools/common.c Fri Mar 30 15:21:42 2007
@@ -163,7 +163,7 @@
N_(" -n show what would be done but don't actually do it\n"),
N_(" -O props SASL security properties\n"),
N_(" -p port port on LDAP server\n"),
-N_(" -P version procotol version (default: 3)\n"),
+N_(" -P version protocol version (default: 3)\n"),
N_(" -Q use SASL Quiet mode\n"),
N_(" -R realm SASL realm\n"),
N_(" -U authcid SASL authentication identity\n"),
@@ -1244,7 +1244,7 @@
crit ? "critical " : "" );
}
- free( ctrls );
+ ldap_controls_free( ctrls );
if ( crit ) {
exit( EXIT_FAILURE );
}
--- openldap-2.3.34/clients/tools/ldapmodify.c.orig Fri Mar 30 15:19:34
2007
+++ openldap-2.3.34/clients/tools/ldapmodify.c Fri Mar 30 15:21:07 2007
@@ -389,8 +389,8 @@
fprintf( rejfp, "\n%s\n", rejbuf );
}
- if (rejfp) free( rejbuf );
- free( rbuf );
+ if (rejfp) ber_memfree( rejbuf );
+ ber_memfree( rbuf );
}
#ifdef LDAP_GROUP_TRANSACTION
@@ -519,7 +519,7 @@
printf(_("%s: skipping change record for entry:
%s\n"),
prog, dn);
printf(_("\t(LDAP host/port does not match
replica: lines)\n"));
- free( dn );
+ ber_memfree( dn );
ber_memfree( type );
ber_memfree( val.bv_val );
return( 0 );
@@ -727,13 +727,13 @@
}
if ( dn != NULL ) {
- free( dn );
+ ber_memfree( dn );
}
if ( newrdn != NULL ) {
- free( newrdn );
+ ber_memfree( newrdn );
}
if ( newsup != NULL ) {
- free( newsup );
+ ber_memfree( newsup );
}
if ( pmods != NULL ) {
ldap_mods_free( pmods, 1 );
16 years, 8 months
(ITS#4900) command-line utils crash
by chris.ridd＠isode.com
Full_Name: Chris Ridd
Version: 2.3.34
OS: Windows
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (62.3.217.250)
On Windows, the ldapadd tool often crashes while parsing the command-line
arguments.
This is due to a defect in the way the optarg (et al) variables are declared in
ac/unistd.h. When tools/common.c includes that file, the 4 variables end up
being declared as being "imported" from a DLL. However they're *actually*
present in a static archive that's linked into the ldapadd executable, which is
apparently not the same sort of thing. (Visual Studio's compiler warns about
inconsistent linkage :-)
As a result, the code in getopt.c thinks that optarg is a variable at one
address, and the code calling getopt() think that optarg is a variable at a
*different* address (NULL in my test rig). Clearly that's not right, and it
causes a variety of issues:
-D <dn> copies optarg using strdup(NULL), which returns NULL.
-w <passwd> scribbles over the passwd string, which crashes doing a *optarg
because optarg is NULL.
A simple fix is to redeclare the 4 variables as just "extern" instead of
LDAP_LIBC_V. Diff follows...
--- openldap-2.3.34/include/ac/unistd.h.orig Fri Mar 30 15:05:49 2007
+++ openldap-2.3.34/include/ac/unistd.h Fri Mar 30 15:06:10 2007
@@ -54,8 +54,8 @@
#else
/* assume we need to declare these externs */
- LDAP_LIBC_V (char *) optarg;
- LDAP_LIBC_V (int) optind, opterr, optopt;
+ extern char * optarg;
+ extern int optind, opterr, optopt;
#endif
/* use lutil file locking */
16 years, 8 months
Re: (ITS#4890) Enhancement: Default Values overlay (patch)
by quanah＠stanford.edu
--On Thursday, March 22, 2007 5:25 PM +0000 docelic(a)mail.inet.hr wrote:
> Full_Name: Davor Ocelic
> Version: CVS-HEAD
> OS: Linux
> URL: http://www.hcoop.net/~docelic/openldap-slapd.overlays.defvals.patch
> Submission from: (NULL) (213.147.110.16)
>
>
>
> Hello,
>
> Attached is a patch which adds a "Default Attribute Values" functionality
> to OpenLDAP. It is implemented as an overlay named "defvals", and the
> patch is done against cvs-head of today.
>
> Complete documentation is included in the slapo-defvals(5) manual page.
> The overlay code itself is commented as well, as a help for others writing
> overlays.
>
> Feel free to trim comments or improve code as you see fit.
Hi Davor,
I believe you'll need to complete the attribution bit detailed here:
<http://www.openldap.org/devel/contributing.html>
for this to be included.
--Quanah
--
Quanah Gibson-Mount
Senior Systems Software Developer
ITS/Shared Application Services
Stanford University
GnuPG Public Key: http://www.stanford.edu/~quanah/pgp.html
16 years, 8 months
Re: (ITS#4898) slapd crashes when no structural object class provided
by BLentz＠channing-bete.com
> --On Wednesday, March 28, 2007 8:45 PM +0000 blentz(a)channing-bete.com
> wrote:
>
> ldbm is deprecated, and it is highly advised that it not be used.
> Bugs with LDBM are unlikely to be fixed, and it has already been
> removed from the 2.4 release. If you can reproduce this issue with
> back-bdb, please follow up.
>
I've removed ldbm support, replacing it with bdb. Everything seems nice
and stable now, and I can't reproduce the crashing with back-bdb.
16 years, 8 months
Re: (ITS#4899) ITS4855 fix breaks 2.3.34
by quanah＠stanford.edu
--On Wednesday, March 28, 2007 11:43 PM +0000 hyc(a)symas.com wrote:
> quanah(a)stanford.edu wrote:
>> Full_Name: Quanah Gibson-Mount
>> Version: 2.3.34
>> OS: Linux 2.6
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (171.66.35.82)
>>
>>
>> I patched 2.3.34 with the patch in RE_23 for fixing the thread pool issue
>> (ITS#4855)
>
> So really this is a bug in RE23, not 2.3.34, yes?
True. :/ I was using my laptop, which is a bit more annoying to type, cut,
and paste with than my normal system ;)
--Quanah
--
Quanah Gibson-Mount
Senior Systems Software Developer
ITS/Shared Application Services
Stanford University
GnuPG Public Key: http://www.stanford.edu/~quanah/pgp.html
16 years, 8 months
(ITS#4899) ITS4855 fix breaks 2.3.34
by quanah＠stanford.edu
Full_Name: Quanah Gibson-Mount
Version: 2.3.34
OS: Linux 2.6
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (171.66.35.82)
I patched 2.3.34 with the patch in RE_23 for fixing the thread pool issue
(ITS#4855)
<http://www.openldap.org/devel/cvsweb.cgi/libraries/libldap_r/tpool.c.diff...>
My patch being:
ldap-uat00:/usr/local/build/PATCHES/openldap# cat ITS4855
--- openldap-2.3.34/libraries/libldap_r/tpool.c.orig 2007-03-26
15:19:56.000000000 -0700
+++ openldap-2.3.34/libraries/libldap_r/tpool.c 2007-03-26 15:20:20.000000000
-0700
@@ -664,7 +664,7 @@
int i;
for ( i=MAXKEYS-1; i>=0; i--) {
- if ( ctx[i].ltk_key )
+ if ( ctx[i].ltk_key == NULL )
continue;
if ( ctx[i].ltk_free )
ctx[i].ltk_free( ctx[i].ltk_key, ctx[i].ltk_data );
Now when I shut down slapd, it core dumps:
1282 cache.c: No such file or directory.
in cache.c
(gdb) bt
#0 bdb_locker_id_free (key=0x2aad2fe00628, data=0x39) at cache.c:1282
#1 0x00002aad2d0bef21 in ldap_pvt_thread_pool_context_reset
(vctx=0x2aad2d1f2260) at tpool.c:670
#2 0x000000000044e07c in slap_destroy () at init.c:304
#3 0x00000000004159f9 in main (argc=3, argv=0x7fff7de07438) at main.c:874
If I remove the patch, there is no core dump.
I see no additional related commits for this change.
--Quanah
16 years, 8 months
