openldap-bugs January 2009

openldap-bugs@openldap.org

50 participants
201 discussions

Re: (ITS#5909) stack overflow in tavl_delete (libraries/liblutil/tavl.c)?
by hyc＠symas.com 29 Jan '09

29 Jan '09

Hardcoding arbitrary limits in portable code is stupid. ("No one will ever need more than 640K...") What may seem (im)practical to you today may be commonplace tomorrow. This code will be maintenance-free no matter what class of machine it's built on, forever. That's *excellent* programming. There is no bug here, this ITS is closed. Ulrich Windl wrote: > On 28 Jan 2009 at 13:06, Howard Chu wrote: > >> Ulrich.Windl(a)rz.uni-regensburg.de wrote: >>> Full_Name: Ulrich Windl >>> Version: 2.4.11 >>> OS: Linux >>> URL: ftp://ftp.openldap.org/incoming/ >>> Submission from: (NULL) (132.199.156.178) >>> >>> >>> Looking for a threaded AVL tree implementation that is supposed to work, I found >>> libraries/liblutil/tavl.c in openLDAP. I'm no expert on threaded AVL trees, but >>> there's thing that worries me: >>> tavl_delete uses two stacks (pptr, pdir) with a size of 8 elements on the >>> routine' stack. While locating the element to delete, it pushes the parent node >>> onto the stack. >> You're misreading the code. The stack is 8*sizeof(void *) which means 32 >> elements deep on a 32 bit machine, and 64 deep on a 64 bit machine. There is >> no possibility of overflow now or for several years to come. > > OK! I was confused by the terrible programming style at that point: as the array > elements are of the proper type (and not bytes), there is no need for a sizeof > here: I see no reason why the depth of the tree to be allowed should depend on the > length of a pointer, maning: If you want 32, why don't you write 32? So the worst > case stack depth would be something like "log2(2^32/(sizeof(void *) * 5))" ("5" > being 2 links + 2 threading marks + 1 data pointer), something near 32-5 == 27 > (for 32-bit machines) and 64-6 == 58. Practically the latter could be safely > reduced to a value around 42 IMHO. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#5909) stack overflow in tavl_delete (libraries/liblutil/tavl.c)?
by ulrich.windl＠rz.uni-regensburg.de 29 Jan '09

29 Jan '09

On 28 Jan 2009 at 13:06, Howard Chu wrote: > Ulrich.Windl(a)rz.uni-regensburg.de wrote: > > Full_Name: Ulrich Windl > > Version: 2.4.11 > > OS: Linux > > URL: ftp://ftp.openldap.org/incoming/ > > Submission from: (NULL) (132.199.156.178) > > > > > > Looking for a threaded AVL tree implementation that is supposed to work, I found > > libraries/liblutil/tavl.c in openLDAP. I'm no expert on threaded AVL trees, but > > there's thing that worries me: > > tavl_delete uses two stacks (pptr, pdir) with a size of 8 elements on the > > routine' stack. While locating the element to delete, it pushes the parent node > > onto the stack. > > You're misreading the code. The stack is 8*sizeof(void *) which means 32 > elements deep on a 32 bit machine, and 64 deep on a 64 bit machine. There is > no possibility of overflow now or for several years to come. OK! I was confused by the terrible programming style at that point: as the array elements are of the proper type (and not bytes), there is no need for a sizeof here: I see no reason why the depth of the tree to be allowed should depend on the length of a pointer, maning: If you want 32, why don't you write 32? So the worst case stack depth would be something like "log2(2^32/(sizeof(void *) * 5))" ("5" being 2 links + 2 threading marks + 1 data pointer), something near 32-5 == 27 (for 32-bit machines) and 64-6 == 58. Practically the latter could be safely reduced to a value around 42 IMHO. > > > So from my knowledge this means that for a perfectly filled balanced tree, the > > stack will overflow when the tree consists of 2^8 (2^0 + 2^1 + 2^2 + ... + 2^7) > > or more nodes, and you are locating a leaf node. Maybe even with fewer nodes. > > As the algorithm also pushes an other element onto the stack, the problem could > > appear even before 128 nodes. > > -- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: slapo-unique man page misleading (ITS#5908)
by quanah＠zimbra.com 28 Jan '09

28 Jan '09

--On January 28, 2009 10:39:41 PM +0000 Magne.Land(a)citrix.com wrote: > Hi, > Please see this thread for background info: > http://www.openldap.org/lists/openldap-software/200901/msg00112.html > > To me, something is either guaranteed or not. And I think "enforce" means > something that is guaranteed. In this case, the uniqueness is not > guaranteed, therefore it is not enforced. Set up slapo-accesslog on your master. Then uniqueness is guaranteed. --Quanah -- Quanah Gibson-Mount Principal Software Engineer Zimbra, Inc -------------------- Zimbra :: the leader in open source messaging and collaboration

1 0

(ITS#5911) password policy - alternate lockout mechanism
by aravind＠freeshell.org 28 Jan '09

28 Jan '09

Full_Name: Aravind Gottipati Version: 2.4.13 OS: Linux - RHEL5 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (63.245.220.241) I'd like to propose a change to how the password lockouts work. The current system does not differentiate between multiple bind attempts with a single (or even a few) incorrect password(s) vs. multiple bind attempts with different incorrect passwords. In our case, this results in a ton of false positives when folks change their password, but don't propagate their password change to all the applications/machines that use it. This causes a bunch of un-necessary lockouts. A real crack attempt on the other hand would most likely try a bunch of passwords (none of which repeat). I have posted the same on the openldap-software mailing lists and Jeff Clowser proposed a scheme that should work to solve the problem. Record each failed bind attempt as a (hash,timestamp) pair. If there is another failed attempt, check the password against these (hash, timestamp) pairs and update the timestamp if the hash is found. If its a new password that hasn't been attempted before, then create a new (hash,timestamp) pair. Lock the account out if there are more than pwdMaxFailure hashes stored. http://www.openldap.org/lists/openldap-software/200901/msg00147.html

1 0

Re: slapo-unique man page misleading (ITS#5908)
by Magne.Land＠citrix.com 28 Jan '09

28 Jan '09

Hi, Please see this thread for background info: http://www.openldap.org/lists/openldap-software/200901/msg00112.html To me, something is either guaranteed or not. And I think "enforce" means something that is guaranteed. In this case, the uniqueness is not guaranteed, therefore it is not enforced. Regards, Magne Land On 1/28/09 2:33 PM, "Gavin Henry" <openldap-its(a)OpenLDAP.org> wrote: > Full_Name: Magne Land > Version: 2.3.43 > OS: RHEL 5.1 64-bit > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (72.37.244.20) > > > Currently the man page of slapo-unique says "enforce uniqueness", which I find > misleading. > Would it be possible to change it to say "makes the best effort to enforce > uniqueness" or something to that effect? > > It would need more words than that to comment on what exactly happens i.e. why it is a "best effort".

1 0

Re: (ITS#5909) stack overflow in tavl_delete (libraries/liblutil/tavl.c)?
by hyc＠symas.com 28 Jan '09

28 Jan '09

Ulrich.Windl(a)rz.uni-regensburg.de wrote: > Full_Name: Ulrich Windl > Version: 2.4.11 > OS: Linux > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (132.199.156.178) > > > Looking for a threaded AVL tree implementation that is supposed to work, I found > libraries/liblutil/tavl.c in openLDAP. I'm no expert on threaded AVL trees, but > there's thing that worries me: > tavl_delete uses two stacks (pptr, pdir) with a size of 8 elements on the > routine' stack. While locating the element to delete, it pushes the parent node > onto the stack. You're misreading the code. The stack is 8*sizeof(void *) which means 32 elements deep on a 32 bit machine, and 64 deep on a 64 bit machine. There is no possibility of overflow now or for several years to come. > So from my knowledge this means that for a perfectly filled balanced tree, the > stack will overflow when the tree consists of 2^8 (2^0 + 2^1 + 2^2 + ... + 2^7) > or more nodes, and you are locating a leaf node. Maybe even with fewer nodes. > As the algorithm also pushes an other element onto the stack, the problem could > appear even before 128 nodes. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

(ITS#5910) Minor fixes to test scripts
by rein＠OpenLDAP.org 28 Jan '09

28 Jan '09

Full_Name: Rein Tollevik Version: CVS HEAD OS: Irrelevant URL: Submission from: (NULL) (81.93.160.250) Submitted by: rein test006 truncates its log file instead of appending to it. test048 substitutes pathnames wrong if it happens to be run from a directory which includes ".2." in the path. Fixes are coming.. Rein Tollevik Basefarm AS

1 0

Re: (ITS#5340) REP_ENTRY_MODIFIABLE bug in dynlist
by hyc＠symas.com 28 Jan '09

28 Jan '09

h.b.furuseth(a)usit.uio.no wrote: > ando(a)sys-net.it writes: >> OK, makes sense. But perhaps creating entries on the stack could be >> deprecated now that entries are pooled and reused by calling >> entry_alloc() entry_free(). This is just food for thoughts, we could as >> well leave all those flags 'round, if we can streamline their handling. > > I think we might as well leave them. Since most are for saving memory, > might even want to expand them eventually. (E.g. separate flags for > _which_ parts of an entry are modifiable. Modifiable attribute list, > DNs, values _in_ attribute list, etc.) > > For that matter, looking at entry_free/entry_alloc, I don't see how that > pooling saves more than it costs. It saves 1 malloc for the Entry > structure, but not the mallocs for the DN and attribute list. And it > costs two global mutex lock/unlocks per entry, in entry_alloc() and > entry_free(). An entry on the stack avoids both malloc and locks. Yes, using the stack is still preferable when it's doable. The point of entry_alloc() was to avoid heap fragmentation, not to generally save on mallocs. It might make sense to turn the entry lists into per-thread lists, to avoid locks in the default cases. But if you follow this path to completion, that turns into re-implementing tcmalloc ("tc" stands for "thread-caching" which is exactly what we would be duplicating...). As for the attribute list: attr_alloc() is also pre-allocated for the same reason. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

(ITS#5909) stack overflow in tavl_delete (libraries/liblutil/tavl.c)?
by Ulrich.Windl＠rz.uni-regensburg.de 27 Jan '09

27 Jan '09

Full_Name: Ulrich Windl Version: 2.4.11 OS: Linux URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (132.199.156.178) Looking for a threaded AVL tree implementation that is supposed to work, I found libraries/liblutil/tavl.c in openLDAP. I'm no expert on threaded AVL trees, but there's thing that worries me: tavl_delete uses two stacks (pptr, pdir) with a size of 8 elements on the routine' stack. While locating the element to delete, it pushes the parent node onto the stack. So from my knowledge this means that for a perfectly filled balanced tree, the stack will overflow when the tree consists of 2^8 (2^0 + 2^1 + 2^2 + ... + 2^7) or more nodes, and you are locating a leaf node. Maybe even with fewer nodes. As the algorithm also pushes an other element onto the stack, the problem could appear even before 128 nodes. If the problem occurs, I'd suspect data corruption at least, because one stack will overwrite the other stack. I've spotted these uses (besides /libraries/liblutil/testtavl.c) of tavl_delete(): servers/slapd/overlays/translucent.c, servers/slapd/overlays/pcache.c I don't understand that code that much, but if the TAVL isn't used in a very controlled way (i.e. limiting the number of possible nodes), a problem may be triggered. I also noticed that the limitation in the code exists for some time already.

1 0

(ITS#5908) slapo-unique man page misleading
by magne.land＠citrix.com 27 Jan '09

27 Jan '09

Full_Name: Magne Land Version: 2.3.43 OS: RHEL 5.1 64-bit URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (72.37.244.20) Currently the man page of slapo-unique says "enforce uniqueness", which I find misleading. Would it be possible to change it to say "makes the best effort to enforce uniqueness" or something to that effect?

1 0

← Newer
1
2
3
4
5
6
7
...
21
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs January 2009