Full_Name: Elia Pinto Version: 2.4 master OS: Linux FC12 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (2605:4400:1:781:216:3eff:fe31:f4d4) From a8ff21429c29f1d2b6ef8f58ec84b7a9036cea73 Mon Sep 17 00:00:00 2001 From: Elia Pinto <gitter.spiros@gmail.com> Date: Thu, 11 Oct 2012 17:49:06 +0200 Subject: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption Recent versions of Linux libc (later than 5.4.23) and glibc (2.x) include a malloc() implementation which is tunable via environment variables. When MALLOC_CHECK_ is set, a special (less efficient) implementation is used which is designed to be tolerant against simple errors, such as double calls of free() with the same argument, or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_ is set to 3, a diagnostic message is printed on stderr and the program is aborted. Setting the MALLOC_PERTURB_ environment variable causes the malloc functions in libc to return memory which has been wiped and clear memory when it is returned. Of course this does not affect calloc which always does clear the memory. The reason for this exercise is, of course, to find code which uses memory returned by malloc without initializing it and code which uses code after it is freed. valgrind can do this but it's costly to run. The MALLOC_PERTURB_ exchanges the ability to detect problems in 100% of the cases with speed. The byte value used to initialize values returned by malloc is the byte value of the environment value. The value used to clear memory is the bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature. This technique can find hard to detect bugs. It is therefore suggested to always use this flag (at least temporarily) when testing out code or a new distribution. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> --- tests/run.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/run.in b/tests/run.in index 5e6178b..fe25d0c 100644 --- a/tests/run.in +++ b/tests/run.in @@ -241,6 +241,11 @@ fi # disable LDAP initialization LDAPNOINIT=true; export LDAPNOINIT +# Add libc malloc_check and MALLOC_PERTURB test +MALLOC_CHECK_=3 +export MALLOC_CHECK_ +MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)" +export MALLOC_PERTURB_ echo "Running ${SCRIPT} for ${BACKEND}..." while [ $COUNTER -le $LOOP ]; do -- 1.7.11.rc1
You can also pull from git://github.com/devzero2000/openldap.git 2012/10/11 <openldap-its@openldap.org>: > > *** THIS IS AN AUTOMATICALLY GENERATED REPLY *** > > Thanks for your report to the OpenLDAP Issue Tracking System. Your > report has been assigned the tracking number ITS#7415. > > One of our support engineers will look at your report in due course. > Note that this may take some time because our support engineers > are volunteers. They only work on OpenLDAP when they have spare > time. > > If you need to provide additional information in regards to your > issue report, you may do so by replying to this message. Note that > any mail sent to openldap-its@openldap.org with (ITS#7415) > in the subject will automatically be attached to the issue report. > > mailto:openldap-its@openldap.org?subject=(ITS#7415) > > You may follow the progress of this report by loading the following > URL in a web browser: > http://www.OpenLDAP.org/its/index.cgi?findid=7415 > > Please remember to retain your issue tracking number (ITS#7415) > on any further messages you send to us regarding this report. If > you don't then you'll just waste our time and yours because we > won't be able to properly track the report. > > Please note that the Issue Tracking System is not intended to > be used to seek help in the proper use of OpenLDAP Software. > Such requests will be closed. > > OpenLDAP Software is user supported. > http://www.OpenLDAP.org/support/ > > -------------- > Copyright 1998-2007 The OpenLDAP Foundation, All Rights Reserved. >
These vars can be set directly from the test environment, no need to modify the test scripts. p. > Full_Name: Elia Pinto > Version: 2.4 master > OS: Linux FC12 > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (2605:4400:1:781:216:3eff:fe31:f4d4) > > >>From a8ff21429c29f1d2b6ef8f58ec84b7a9036cea73 Mon Sep 17 00:00:00 2001 > From: Elia Pinto <gitter.spiros@gmail.com> > Date: Thu, 11 Oct 2012 17:49:06 +0200 > Subject: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the > test > suite for detecting heap corruption > > Recent versions of Linux libc (later than 5.4.23) and glibc (2.x) > include a malloc() implementation which is tunable via environment > variables. When MALLOC_CHECK_ is set, a special (less efficient) > implementation is used which is designed to be tolerant against > simple errors, such as double calls of free() with the same argument, > or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_ > is set to 3, a diagnostic message is printed on stderr > and the program is aborted. > > Setting the MALLOC_PERTURB_ environment variable causes the malloc > functions in libc to return memory which has been wiped and clear > memory when it is returned. > Of course this does not affect calloc which always does clear the memory. > > The reason for this exercise is, of course, to find code which uses > memory returned by malloc without initializing it and code which uses > code after it is freed. valgrind can do this but it's costly to run. > The MALLOC_PERTURB_ exchanges the ability to detect problems in 100% > of the cases with speed. > > The byte value used to initialize values returned by malloc is the byte > value of the environment value. The value used to clear memory is the > bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature. > > This technique can find hard to detect bugs. > It is therefore suggested to always use this flag (at least temporarily) > when testing out code or a new distribution. > > Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> > --- > tests/run.in | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tests/run.in b/tests/run.in > index 5e6178b..fe25d0c 100644 > --- a/tests/run.in > +++ b/tests/run.in > @@ -241,6 +241,11 @@ fi > > # disable LDAP initialization > LDAPNOINIT=true; export LDAPNOINIT > +# Add libc malloc_check and MALLOC_PERTURB test > +MALLOC_CHECK_=3 > +export MALLOC_CHECK_ > +MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)" > +export MALLOC_PERTURB_ > > echo "Running ${SCRIPT} for ${BACKEND}..." > while [ $COUNTER -le $LOOP ]; do > -- > 1.7.11.rc1 > > > > > > -- Pierangelo Masarati Associate Professor Dipartimento di Ingegneria Aerospaziale Politecnico di Milano
It is true in general, but it is also a matter of a personal taste. If the openldap project use a buildboot that set in the environment these variables , that patch is useless. This is also partially true if the openldap developer use to set these variables in their personal environment, because other contributor could not do the same. Other project use a similar patch , for example popt 1.17 devel and git 1.8 in master, i am the author so i know . what is more openldap now are not using automake and the little test environment that automake could use, so i have some difficulty to understeand what is this alternative "test environment" . Perhaps i have missed something ? 2012/10/11, Pierangelo Masarati <masarati@aero.polimi.it>: > These vars can be set directly from the test environment, no need to > modify the test scripts. > > p. > >> Full_Name: Elia Pinto >> Version: 2.4 master >> OS: Linux FC12 >> URL: ftp://ftp.openldap.org/incoming/ >> Submission from: (NULL) (2605:4400:1:781:216:3eff:fe31:f4d4) >> >> >>>From a8ff21429c29f1d2b6ef8f58ec84b7a9036cea73 Mon Sep 17 00:00:00 2001 >> From: Elia Pinto <gitter.spiros@gmail.com> >> Date: Thu, 11 Oct 2012 17:49:06 +0200 >> Subject: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the >> test >> suite for detecting heap corruption >> >> Recent versions of Linux libc (later than 5.4.23) and glibc (2.x) >> include a malloc() implementation which is tunable via environment >> variables. When MALLOC_CHECK_ is set, a special (less efficient) >> implementation is used which is designed to be tolerant against >> simple errors, such as double calls of free() with the same argument, >> or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_ >> is set to 3, a diagnostic message is printed on stderr >> and the program is aborted. >> >> Setting the MALLOC_PERTURB_ environment variable causes the malloc >> functions in libc to return memory which has been wiped and clear >> memory when it is returned. >> Of course this does not affect calloc which always does clear the memory. >> >> The reason for this exercise is, of course, to find code which uses >> memory returned by malloc without initializing it and code which uses >> code after it is freed. valgrind can do this but it's costly to run. >> The MALLOC_PERTURB_ exchanges the ability to detect problems in 100% >> of the cases with speed. >> >> The byte value used to initialize values returned by malloc is the byte >> value of the environment value. The value used to clear memory is the >> bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature. >> >> This technique can find hard to detect bugs. >> It is therefore suggested to always use this flag (at least temporarily) >> when testing out code or a new distribution. >> >> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> >> --- >> tests/run.in | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/tests/run.in b/tests/run.in >> index 5e6178b..fe25d0c 100644 >> --- a/tests/run.in >> +++ b/tests/run.in >> @@ -241,6 +241,11 @@ fi >> >> # disable LDAP initialization >> LDAPNOINIT=true; export LDAPNOINIT >> +# Add libc malloc_check and MALLOC_PERTURB test >> +MALLOC_CHECK_=3 >> +export MALLOC_CHECK_ >> +MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)" >> +export MALLOC_PERTURB_ >> >> echo "Running ${SCRIPT} for ${BACKEND}..." >> while [ $COUNTER -le $LOOP ]; do >> -- >> 1.7.11.rc1 >> >> >> >> >> >> > > > -- > Pierangelo Masarati > Associate Professor > Dipartimento di Ingegneria Aerospaziale > Politecnico di Milano > > -- Inviato dal mio dispositivo mobile
I note that these environment variables are specific to Linux, and then only certain Linux systems. If we add these, then we should also add similar environment variables for FreeBSD malloc(3), MacOS/X malloc(3), and various alternative malloc(3) libraries. And then we might get into cases where variables for one set of environmental variables conflict with another. As these can be set by the developer as desired in their development environment, there's little reason to get into setting all the malloc(3) environmental variables that might be useful... and we shouldn't play favors and only set variables for Linux. -- Kurt
And, I should add, setting them up generally (not just for OpenLDAP command line tools), means they catch non-OpenLDAP bugs... and that leads to non-OpenLDAP issues being reported to the OpenLDAP Project. We should avoid this for when 'make test' is simply used by deployers as a quick sanity tools. Developers, on the other hand, hopefully know what they are doing when they set such environmental variables... and they know how to manage this locally. Best to avoid adding such to the public test system. -- Kurt
> It is true in general, but it is also a matter of a personal taste. If > the openldap project use a buildboot that set in the environment these > variables , that patch is useless. This is also partially true if the > openldap developer use to set these variables in their personal > environment, because other contributor could not do the same. Other > project use a similar patch , for example popt 1.17 devel and git 1.8 > in master, i am the author so i know . what is more openldap now are > not using automake and the little test environment that automake could > use, so i have some difficulty to understeand what is this alternative > "test environment" . Perhaps i have missed something ? I meant the environment of the shell tests are run from. IMHO, I'd consider setting those macros a developer's conscious choice rather than a normal user's act. Moreover, they're specific to glibc, while users might want to use different malloc implementations, or build on platforms where glibc is not available, so the fact they're not honored might not be obvious. My 2c, of course. p. -- Pierangelo Masarati Associate Professor Dipartimento di Ingegneria Aerospaziale Politecnico di Milano
We can describe these and other variables in tests/README. Yeah I know, people don't read documentation. The testing announcements could mention the file though.
On Oct 11, 2012, at 1:44 PM, h.b.furuseth@usit.uio.no wrote: > We can describe these and other variables in tests/README. There's lots of malloc variables one could describe. Better off to simply say that one might want to read the documentation for whatever malloc they are using, and use, if they want, any environmental variables they might want to use. This avoids duplicating detail better left to the malloc documentation. > > Yeah I know, people don't read documentation. The testing > announcements could mention the file though. Wrong audience, me thinks, especially if those announcements are posted to openldap-techncial. We don't need openldap users reporting malloc use issues in non-OpenLDAP programs. Use of such variables should be limited to developers. -- Kurt > >
changed notes changed state Open to Closed
2012/10/11 Kurt Zeilenga <Kurt@openldap.org>: > I note that these environment variables are specific to Linux, and then only certain Linux systems. If we add these, then we should also add similar environment variables for FreeBSD malloc(3), MacOS/X malloc(3), and various alternative malloc(3) libraries. And then we might get into cases where variables for one set of environmental variables conflict with another. As these can be set by the developer as desired in their development environment, there's little reason to get into setting all the malloc(3) environmental variables that might be useful... and we shouldn't play favors and only set variables for Linux. Sorry for the delay, problems to the my $DAYWORK Ok. First of all, elibc (debian/ubuntu recently) and glibc cover probably 95% of the Linux distributions, and they are compatible, in particular for the MALLOC_CHECK http://lwn.net/Articles/333755/ - i have also checked the source. A considerable part of the remaining operating systems are like BSD - FreeBSD for example - and MacOSX, FreeBSD based. In fact, these latter two have different malloc library and it may be necessary to activate different controls for different systems (see the first reference) http://permalink.gmane.org/gmane.network.openvswitch.devel/14838 http://www.unix.com/man-page/freebsd/5/malloc.conf/ But the benefits of automating these tests is to do a better AUTOQA for the buildsystem. It is also not true, as experience shows, that all developers know everything. Beeing expert in LDAP and C programming, for example, does not mean necessarily be an expert in everything: this can certainly be true for the OPENLDAP developers but it is not always the case. Here some examples: http://lists.fedoraproject.org/pipermail/devel/2010-May/135683.html (this check had found many bug that the upstream developer haven't catched) or this, from my mantainer, http://lists.gnupg.org/pipermail/gcrypt-devel/2010-June/001605.html I saw that you have closed the case(WANTFIX i think), however, I have to tell you better my opinion. If, as can happen, you decide that my reasons make sense, i can reroll my patch for Linux, macos and freebsd. Let me know. Thanks anyway for your reply > > -- Kurt >
You miss the fact that we encourage deployers of OpenLDAP Software, who we recommend build OpenLDAP Software from source, run 'make test' before they 'make install'. We don't want these deployers to have false positives, such as would be likely caused if we added such environmental variables. For those doing automated checks, such as those who do construct packages, they can have local patches to their hearts content. Likewise for developers. So, if it was up to me, I would reject your patch as, IMO, it's in appropriate for our source distributions. I suspect Howard will chime in sooner or later. -- Kurt
2012/10/12 Kurt Zeilenga <Kurt@openldap.org>: > You miss the fact that we encourage deployers of OpenLDAP Software, who we recommend build OpenLDAP Software from source, run 'make test' before they 'make install'. We don't want these deployers to have false positives, such as would be likely caused if we added such environmental variables. It is your own opinion, right? Have experiences in this regard? I do not. I personally run make test on openldap with Ubuntu 4.12, Fedora 17 and RHEL6 and my patch without any problem (good for openldap :=) ). And many other projects, for example, git hat has a very extensive test suite, use MALLOC_CHECK as in my patch, integrated in a test suite. However, I will not insist further on a trivial patch. Also because this discussion gave me the opportunity to investigate better the issue, and I'll just patch for FreeBSD my software. So, thank you anyway for the useful observations. Regards > > For those doing automated checks, such as those who do construct packages, they can have local patches to their hearts content. Likewise for developers. > > So, if it was up to me, I would reject your patch as, IMO, it's in appropriate for our source distributions. I suspect Howard will chime in sooner or later. > > -- Kurt >
On Oct 12, 2012, at 8:38 AM, Elia Pinto <gitter.spiros@gmail.com> wrote: > 2012/10/12 Kurt Zeilenga <Kurt@openldap.org>: >> You miss the fact that we encourage deployers of OpenLDAP Software, who we recommend build OpenLDAP Software from source, run 'make test' before they 'make install'. We don't want these deployers to have false positives, such as would be likely caused if we added such environmental variables. > > It is your own opinion, right? Have experiences in this regard? Yes and Yes. I use to run with such environmental variables set in my .login. Not anymore, too many odd crashes. So sometimes I do something like: env MallocScribble=yes MallocErrorAbort=yes make test but I've that crash above and below the run script, but not in OpenLDAP Software itself. So now, because I tend to run stuff I'm working on in a debugger, I now mostly rely on my debugger to turn malloc debugging on. And I tend to use other tools for leak detection (like leaks(1) on MacOS/X). -- Kurt > I do > not. I personally run make test on openldap with Ubuntu 4.12, Fedora > 17 and RHEL6 and my patch without any problem (good for openldap :=) > ). And many other projects, for example, git hat has a very extensive > test suite, use MALLOC_CHECK as in my patch, integrated in a test > suite. > > However, I will not insist further on a trivial patch. Also because > this discussion gave me the opportunity to investigate better the > issue, and I'll just patch for FreeBSD my software. So, thank you > anyway for the useful observations. > > Regards >> >> For those doing automated checks, such as those who do construct packages, they can have local patches to their hearts content. Likewise for developers. >> >> So, if it was up to me, I would reject your patch as, IMO, it's in appropriate for our source distributions. I suspect Howard will chime in sooner or later. >> >> -- Kurt >>
2012/10/12 Kurt Zeilenga <Kurt@openldap.org>: > > On Oct 12, 2012, at 8:38 AM, Elia Pinto <gitter.spiros@gmail.com> wrote: > >> 2012/10/12 Kurt Zeilenga <Kurt@openldap.org>: >>> You miss the fact that we encourage deployers of OpenLDAP Software, who we recommend build OpenLDAP Software from source, run 'make test' before they 'make install'. We don't want these deployers to have false positives, such as would be likely caused if we added such environmental variables. >> >> It is your own opinion, right? Have experiences in this regard? > > Yes and Yes. I use to run with such environmental variables set in my .login. Not anymore, too many odd crashes. So sometimes I do something like: > env MallocScribble=yes MallocErrorAbort=yes make test > > but I've that crash above and below the run script, but not in OpenLDAP Software itself. So now, because I tend to run stuff I'm working on in a debugger, I now mostly rely on my debugger to turn malloc debugging on. And I tend to use other tools for leak detection (like leaks(1) on MacOS/X). HeHe, i suspect was ONLY a MacOS/X crap. Good to know. > > -- Kurt > > >> I do >> not. I personally run make test on openldap with Ubuntu 4.12, Fedora >> 17 and RHEL6 and my patch without any problem (good for openldap :=) >> ). And many other projects, for example, git hat has a very extensive >> test suite, use MALLOC_CHECK as in my patch, integrated in a test >> suite. >> >> However, I will not insist further on a trivial patch. Also because >> this discussion gave me the opportunity to investigate better the >> issue, and I'll just patch for FreeBSD my software. So, thank you >> anyway for the useful observations. >> >> Regards >>> >>> For those doing automated checks, such as those who do construct packages, they can have local patches to their hearts content. Likewise for developers. >>> >>> So, if it was up to me, I would reject your patch as, IMO, it's in appropriate for our source distributions. I suspect Howard will chime in sooner or later. >>> >>> -- Kurt >>> >
Kurt@OpenLDAP.org wrote: > You miss the fact that we encourage deployers of OpenLDAP Software, who we recommend build OpenLDAP Software from source, run 'make test' before they 'make install'. We don't want these deployers to have false positives, such as would be likely caused if we added such environmental variables. > > For those doing automated checks, such as those who do construct packages, they can have local patches to their hearts content. Likewise for developers. > > So, if it was up to me, I would reject your patch as, IMO, it's in appropriate for our source distributions. I suspect Howard will chime in sooner or later. I think you've already covered the points. I have plenty of checks like this in my local source tree, but I see no reason to embed OS and platform-specific stuff into the distributed source. I believe it would lend a false sense of security, particularly when you're on a platform that ignores these variables, but you aren't aware of the fact. It is always the developers' responsibility to know how their own development environment works, and to tweak it to suit their development efforts. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
not needed