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/
--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
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
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".
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/
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
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/
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.
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?
hyc(a)symas.com wrote:
> mbackes(a)symas.com wrote:
>> Full_Name: Matthew Backes
>> Version: 2.4, head
>> OS: any
>> URL:
>> Submission from: (NULL) (76.88.99.93)
>>
>>
>> If slapo-memberof is instanced more than once, deleting the
>> member-attr values from a group or deleting the group object will not
>> remove the memberof-attr values from the members.
>>
>> Adds are not affected.
>>
>> Internally, the operation fails because it tries using the
>> memberof-attribute name from the last memberof instance in the stack.
>
> The memberOf code is using a single static variable to hold its state. This is
> obviously broken. It's also using thread keys for its temporary storage, which
> is totally unnecessary; it should simply use its own callback and the
> cb_private field of that callback. As such, much of this code needs to be
> gutted and rewritten.
See also ITS#5654.
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
-----------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Fax: +39 0382 476497
Email: ando(a)sys-net.it
-----------------------------------