Full_Name: Raphael Ouazana Version: HEAD OS: Linux URL: ftp://ftp.openldap.org/incoming/raphael-ouazana-070307.patch Submission from: (NULL) (82.225.231.106) Hi, This patch creates a new operator for sets : /- It allows to get the nth parent of all the elements of a set. For example, if user is cn=user,ou=people,dc=example,dc=com : user/-1 : resolves to set {ou=people,dc=example,dc=com} user/-2 : resolves to set {dc=example,dc=com} Of course it works the same for sets of more than 1 element. It is very useful when ACLs depends on hierarchy. Legal notice : This patch file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in this following patch were developed by Raphael Ouazana raphael.ouazana@linagora.com. These modifications are not subject to any license of Linagora. The attached modifications to OpenLDAP Software are subject to the following notice: Copyright 2007 Raphael Ouazana, Linagora Redistribution and use in source and binary forms, with or without modification, are permitted only as authorized by the OpenLDAP Public License.
raphael.ouazana@linagora.com wrote: I see at least 2 issues in this patch: 1) the use of dnParent() in place on malloc'd data would lead to memory corruption, because at some point the value of the set will be freed, but the bv_val of that value would no longer point to the beginning of a malloc()'ed block. You should rather use AC_MEMCPY() to move the parent's DN at the beginning of the malloc()'ed memory block. 2) when trimming a set of DNs to some ancestor, DNs with common ancestors could result in duplicate set values. I'm not sure this currently has any adverse effect on set evaluation, but I'd consider it bad anyway. You should eliminate duplicates, in case any arises. p. Ing. Pierangelo Masarati OpenLDAP Core Team SysNet s.n.c. Via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it ------------------------------------------ Office: +39.02.23998309 Mobile: +39.333.4963172 Email: pierangelo.masarati@sys-net.it ------------------------------------------
changed notes changed state Open to Feedback moved from Incoming to Contrib
On Jeu 8 mars 2007 01:00, Pierangelo Masarati wrote: > raphael.ouazana@linagora.com wrote: > > I see at least 2 issues in this patch: > > 1) the use of dnParent() in place on malloc'd data would lead to memory > corruption, because at some point the value of the set will be freed, > but the bv_val of that value would no longer point to the beginning of a > malloc()'ed block. You should rather use AC_MEMCPY() to move the > parent's DN at the beginning of the malloc()'ed memory block. > > 2) when trimming a set of DNs to some ancestor, DNs with common > ancestors could result in duplicate set values. I'm not sure this > currently has any adverse effect on set evaluation, but I'd consider it > bad anyway. You should eliminate duplicates, in case any arises. Fixing these issues, I saw that set_parent will be very similar to set_chase. Do you think I should create a new gatherer instead of ? Regards, Raphael Ouazana.
raphael.ouazana@linagora.com wrote: > Fixing these issues, I saw that set_parent will be very similar to > set_chase. Do you think I should create a new gatherer instead of ? I don't see so much similarity: set_chase() is intended to use the gatherer to fetch objects and extract a value from them, including "closure" (recursion). Sublevel does not require fetching, and takes a negative digit as extra argument, rather than an attribute type, so you'd need to add an outer test before calling set_chase() anyway, extend the interface of set_chase() to allow an AttributeDescription and an integer, and so on, while you don't need any callback capability to fetch an object or expand values or so. I'd keep it simple as it is now. p. Ing. Pierangelo Masarati OpenLDAP Core Team SysNet s.n.c. Via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it ------------------------------------------ Office: +39.02.23998309 Mobile: +39.333.4963172 Email: pierangelo.masarati@sys-net.it ------------------------------------------
Le Lun 12 mars 2007 06:37, Pierangelo Masarati a écrit : > I don't see so much similarity: set_chase() is intended to use the > gatherer to fetch objects and extract a value from them, including > "closure" (recursion). Sublevel does not require fetching, and takes a > negative digit as extra argument, rather than an attribute type, so > you'd need to add an outer test before calling set_chase() anyway, > extend the interface of set_chase() to allow an AttributeDescription and > an integer, and so on, while you don't need any callback capability to > fetch an object or expand values or so. I'd keep it simple as it is now. OK. You'll find a new version here: ftp://ftp.openldap.org/incoming/raphael-ouazana-070313.patch It takes care of your notices. Regards, Raphael Ouazana.
Hi, I have uploaded a new version of this patch: ftp://ftp.openldap.org/incoming/raphael-ouazana-070906.patch Now /-0 allows to get all the parents of an entry. So to keep the example where user is cn=user,ou=people,dc=example,dc=com: user/-0 : resolves to set { "cn=user,ou=people,dc=example,dc=com" , "ou=people,dc=example,dc=com", "dc=example,dc=com" , "dc=com" , "" } In fact it was already possible to do that with sets with filters using entryDN:dnSuperiorMatch, but it was really slower (in 2.3 as in HEAD). Do you think this patch has any chance to be accepted for future 2.4? Regards, Raphael Ouazana. Legal notice : This patch file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in this following patch were developed by Raphael Ouazana raphael.ouazana@linagora.com. These modifications are not subject to any license of Linagora. The attached modifications to OpenLDAP Software are subject to the following notice: Copyright 2007 Raphael Ouazana, Linagora Redistribution and use in source and binary forms, with or without modification, are permitted only as authorized by the OpenLDAP Public License.
raphael.ouazana@linagora.com wrote: > Do you think this patch has any chance to be accepted for future 2.4? Applied (with minor changes) to HEAD, please test. And, could you document it on the FAQ, please? One improvement that could be applied is to allow numbers larger than 9... 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
Le Lun 10 septembre 2007 00:12, Pierangelo Masarati a écrit : > raphael.ouazana@linagora.com wrote: > >> Do you think this patch has any chance to be accepted for future 2.4? > > Applied (with minor changes) to HEAD, please test. It seems OK with HEAD, but only if I revert this patch: http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/sets.c.diff?r1=1.28.2.1&r2=1.28.2.2&hideattic=1&sortbydate=0&f=h With this patch, I get a segfault. > And, could you > document it on the FAQ, please? Done: http://www.openldap.org/faq/data/cache/1133.html. Does it seems good for you ? > One improvement that could be applied > is to allow numbers larger than 9... As far as I know, there is no such limitation... Regards, Raphael Ouazana.
raphael.ouazana@linagora.com wrote: > Le Lun 10 septembre 2007 00:12, Pierangelo Masarati a écrit : >> raphael.ouazana@linagora.com wrote: >> >>> Do you think this patch has any chance to be accepted for future 2.4? >> Applied (with minor changes) to HEAD, please test. > > It seems OK with HEAD, but only if I revert this patch: > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/sets.c.diff?r1=1.28.2.1&r2=1.28.2.2&hideattic=1&sortbydate=0&f=h > > With this patch, I get a segfault. I think that's somehow related to ITS#4873, which I should definitely take time to solve. Unfortunately, with the proposed patch I see a leak, and I couldn't find time to track it down. > >> And, could you >> document it on the FAQ, please? > > Done: http://www.openldap.org/faq/data/cache/1133.html. Does it seems > good for you ? > >> One improvement that could be applied >> is to allow numbers larger than 9... > > As far as I know, there is no such limitation... Then I probably misinterpreted that bit of parsing. 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
raphael.ouazana@linagora.com wrote: > It seems OK with HEAD, but only if I revert this patch: > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/sets.c.diff?r1=1.28.2.1&r2=1.28.2.2&hideattic=1&sortbydate=0&f=h > > With this patch, I get a segfault. I have just committed a cleanup of the slap_set_join() function that should be consistent. It should fix a leak in case of '&' on overlapping sets, and consistently handle memory. Can you please test it and point out failures? If you get any, please post the rules that cause them, as those I could design worked fine (tested with valgrind). > >> And, could you >> document it on the FAQ, please? > > Done: http://www.openldap.org/faq/data/cache/1133.html. Does it seems > good for you ? Well, I'd prefer you to merge your comments with the existing, giant one. The contents look fine (although I'm not a native English speaker), except for one consideration: for consistency, "/-0" should return the DN untouched (although useless); perhaps "/-*" or something like that could be used to explode the DN into all ancestors. 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
changed notes changed state Feedback to Test moved from Contrib to Software Enhancements
moved from Software Enhancements to Development
ando@sys-net.it a écrit : > raphael.ouazana@linagora.com wrote: > >> It seems OK with HEAD, but only if I revert this patch: >> http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/sets.c.diff?r1=1.28.2.1&r2=1.28.2.2&hideattic=1&sortbydate=0&f=h >> >> With this patch, I get a segfault. > > I have just committed a cleanup of the slap_set_join() function that > should be consistent. It should fix a leak in case of '&' on > overlapping sets, and consistently handle memory. Can you please test > it and point out failures? If you get any, please post the rules that > cause them, as those I could design worked fine (tested with valgrind). I am one of Raphael's colleagues, answering on his behalf. I've tested your latest commit, and most of our tests now run great. However, I still get a segault with the two rules below. Please note that this segfault only happens when *both* rules are present, each one by itself does not cause a segfault : > access to dn.sub="ou=Affectations,dc=linagora,dc=org" attrs=sigleAbrege,labeledURI,mailRoutingAddress,telephoneNumber,facsimileTelephoneNumber,entry > by set="([ldap:///] + (([ldap:///] + ((([ldap:///] + this + [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-0) + [??base?])/responsable) + [??base?(|(administrateurResponsable=] + user + [)(administrateur=] + user + [)(membre=] + user + [))])/entryDN" +rscx > by * break > access to dn.sub="ou=Affectations,dc=linagora,dc=org" attrs=domaineMessagerie,finValidite,identifiantHarpegeStructure,responsable,objectClass,entry > by set="[ldap:///] + (((([ldap:///] + this + [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-100) & ((([ldap:///] + user + [??base?(objectClass=personnel)])/entryDN)/-100)) + [??sub?entryDN=] + user/entryDN" +rscx > by * break I'm afraid that we use quite a few specific schemas so you may not be able to reproduce this easily. However, I hope these rules will enable you to determine the problematic case. If necessary, I could prepare a data and schema extract to reproduce the problem. Regards, Jon -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
> I've tested your latest commit, and most of our tests now run great. > However, I still get a segault with the two rules below. Please note > that this segfault only happens when *both* rules are present, each one > by itself does not cause a segfault : > >> access to dn.sub="ou=Affectations,dc=linagora,dc=org" >> attrs=sigleAbrege,labeledURI,mailRoutingAddress,telephoneNumber,facsimileTelephoneNumber,entry >> by set="([ldap:///] + (([ldap:///] + ((([ldap:///] + this + >> [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-0) >> + [??base?])/responsable) + >> [??base?(|(administrateurResponsable=] + user + >> [)(administrateur=] + user + [)(membre=] + user + [))])/entryDN" >> +rscx >> by * break > >> access to dn.sub="ou=Affectations,dc=linagora,dc=org" >> attrs=domaineMessagerie,finValidite,identifiantHarpegeStructure,responsable,objectClass,entry >> by set="[ldap:///] + (((([ldap:///] + this + >> [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-100) >> & ((([ldap:///] + user + >> [??base?(objectClass=personnel)])/entryDN)/-100)) + >> [??sub?entryDN=] + user/entryDN" +rscx >> by * break > > I'm afraid that we use quite a few specific schemas so you may not be > able to reproduce this easily. However, I hope these rules will enable > you to determine the problematic case. If necessary, I could prepare a > data and schema extract to reproduce the problem. That would help, but before you do it, could you please post a backtrace of the stack when the issue occurs? In case this doesn't suffice, and I can't figure things out myself, I'll ask you to provide a self-contained set of data that causes the issue. 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
jclarke@linagora.com a écrit : > ando@sys-net.it a écrit : >> raphael.ouazana@linagora.com wrote: >> >>> It seems OK with HEAD, but only if I revert this patch: >>> http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/sets.c.diff?r1=1.28.2.1&r2=1.28.2.2&hideattic=1&sortbydate=0&f=h >>> >>> With this patch, I get a segfault. >> I have just committed a cleanup of the slap_set_join() function that >> should be consistent. It should fix a leak in case of '&' on >> overlapping sets, and consistently handle memory. Can you please test >> it and point out failures? If you get any, please post the rules that >> cause them, as those I could design worked fine (tested with valgrind). > > I am one of Raphael's colleagues, answering on his behalf. > > I've tested your latest commit, and most of our tests now run great. > However, I still get a segault with the two rules below. Please note > that this segfault only happens when *both* rules are present, each one > by itself does not cause a segfault : > >> access to dn.sub="ou=Affectations,dc=linagora,dc=org" attrs=sigleAbrege,labeledURI,mailRoutingAddress,telephoneNumber,facsimileTelephoneNumber,entry >> by set="([ldap:///] + (([ldap:///] + ((([ldap:///] + this + [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-0) + [??base?])/responsable) + [??base?(|(administrateurResponsable=] + user + [)(administrateur=] + user + [)(membre=] + user + [))])/entryDN" +rscx >> by * break > >> access to dn.sub="ou=Affectations,dc=linagora,dc=org" attrs=domaineMessagerie,finValidite,identifiantHarpegeStructure,responsable,objectClass,entry >> by set="[ldap:///] + (((([ldap:///] + this + [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-100) & ((([ldap:///] + user + [??base?(objectClass=personnel)])/entryDN)/-100)) + [??sub?entryDN=] + user/entryDN" +rscx >> by * break > > I'm afraid that we use quite a few specific schemas so you may not be > able to reproduce this easily. However, I hope these rules will enable > you to determine the problematic case. If necessary, I could prepare a > data and schema extract to reproduce the problem. Oh, and I forgot, please find attached the output from valgrind when running these rules. Jon -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
Pierangelo Masarati a écrit : >> I've tested your latest commit, and most of our tests now run great. >> However, I still get a segault with the two rules below. Please note >> that this segfault only happens when *both* rules are present, each one >> by itself does not cause a segfault : >> >>> access to dn.sub="ou=Affectations,dc=linagora,dc=org" >>> attrs=sigleAbrege,labeledURI,mailRoutingAddress,telephoneNumber,facsimileTelephoneNumber,entry >>> by set="([ldap:///] + (([ldap:///] + ((([ldap:///] + this + >>> [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-0) >>> + [??base?])/responsable) + >>> [??base?(|(administrateurResponsable=] + user + >>> [)(administrateur=] + user + [)(membre=] + user + [))])/entryDN" >>> +rscx >>> by * break >>> access to dn.sub="ou=Affectations,dc=linagora,dc=org" >>> attrs=domaineMessagerie,finValidite,identifiantHarpegeStructure,responsable,objectClass,entry >>> by set="[ldap:///] + (((([ldap:///] + this + >>> [??base?(|(objectClass=affectationLiee)(objectClass=affectationSemiLibre))])/entryDN)/-100) >>> & ((([ldap:///] + user + >>> [??base?(objectClass=personnel)])/entryDN)/-100)) + >>> [??sub?entryDN=] + user/entryDN" +rscx >>> by * break >> I'm afraid that we use quite a few specific schemas so you may not be >> able to reproduce this easily. However, I hope these rules will enable >> you to determine the problematic case. If necessary, I could prepare a >> data and schema extract to reproduce the problem. > > That would help, but before you do it, could you please post a backtrace > of the stack when the issue occurs? In case this doesn't suffice, and I > can't figure things out myself, I'll ask you to provide a self-contained > set of data that causes the issue. Of course. Please also see my message a few minutes ago with valgrind output. Stack trace follows: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1740719216 (LWP 10412)] 0xb7bcfdec in free () from /lib/tls/i686/cmov/libc.so.6 (gdb) bt #0 0xb7bcfdec in free () from /lib/tls/i686/cmov/libc.so.6 #1 0x0816338e in ber_bvarray_free_x (a=0x82e4058, ctx=0x82bd6b8) at memory.c:731 #2 0x080af40a in slap_set_filter (gatherer=0x8086720 <acl_set_gather>, cp=0x983291e0, fbv=0x983291e8, user=0x82dc094, target=0xb5312bd0, results=0x0) at sets.c:391 #3 0x080864ca in acl_match_set (subj=0x983295b0, op=0x82dbff8, e=0xb5312bc4, default_set_attribute=0x0) at acl.c:2299 #4 0x08089786 in slap_access_allowed (op=0x82dbff8, e=0xb5312bc4, desc=0x8244de8, val=0x97eeb4c0, access=ACL_SEARCH, state=0x0, maskp=0x98329c20) at acl.c:1621 #5 0x0808b6c4 in fe_access_allowed (op=0x82dbff8, e=0xb5312bc4, desc=0x8244de8, val=0x97eeb4c0, access=ACL_SEARCH, state=0x0, maskp=0x98329c20) at acl.c:311 #6 0x080c8930 in over_access_allowed (op=0x82dbff8, e=0xb5312bc4, desc=0x8244de8, val=0x97eeb4c0, access=ACL_SEARCH, state=0x0, maskp=0x98329c20) at backover.c:314 #7 0x080872c4 in access_allowed_mask (op=0x82dbff8, e=0xb5312bc4, desc=0x8244de8, val=0x97eeb4c0, access=<value optimized out>, state=0x0, maskp=0x0) at acl.c:413 #8 0x08083bcf in test_ava_filter (op=0x82dbff8, e=0xb5312bc4, ava=0x97eeb4bc, type=163) at filterentry.c:545 #9 0x080843e9 in test_filter (op=0x82dbff8, e=0xb5312bc4, f=0x97eeb4e4) at filterentry.c:88 #10 0x080dc90e in bdb_search (op=0x82dbff8, rs=0x983eb154) at search.c:844 #11 0x080679f6 in fe_op_search (op=0x82dbff8, rs=0x983eb154) at search.c:368 #12 0x080c7ca1 in overlay_op_walk (op=0x82dbff8, rs=0x983eb154, which=op_search, oi=0x8264bf8, on=0x8264cf8) at backover.c:652 #13 0x080c820d in over_op_func (op=0x82dbff8, rs=0x983eb154, which=op_search) at backover.c:704 #14 0x0806823e in do_search (op=0x82dbff8, rs=0x983eb154) at search.c:217 #15 0x0806583e in connection_operation (ctx=0x983eb238, arg_v=0x82dbff8) at connection.c:1145 #16 0x08065c71 in connection_read_thread (ctx=0x983eb238, argv=0xe) at connection.c:1271 #17 0x0813cda6 in ldap_int_thread_pool_wrapper (xpool=0x824bfd0) at tpool.c:614 #18 0xb7cab31b in start_thread () from /lib/tls/i686/cmov/libpthread.so.0 #19 0xb7c3457e in clone () from /lib/tls/i686/cmov/libc.so.6 Regards, Jon -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
changed notes changed state Test to Release
Should be fixed now in HEAD/re24/re23. Please test. 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
Hi, Le Jeu 13 septembre 2007 22:47, ando@sys-net.it a écrit : > Should be fixed now in HEAD/re24/re23. Please test. p. Thank you for that and for the various fixes (/-*, documentation). We are going to test your fixes as soon as possible. Regards, Raphael Ouazana.
Pierangelo Masarati a écrit : > Should be fixed now in HEAD/re24/re23. Please test. p. Hi, I've been testing (at last, sorry for the delay), and I've come across another memory problem. Backtrace is below, and valgrind output is attached. Please shout if you need any more information. Backtrace follows: 8<------------------------------------------------------------------- conn=0 op=0 BIND dn="" method=128 conn=0 op=0 RESULT tag=97 err=0 text= conn=0 op=1 SRCH base="ou=people,dc=linagora,dc=org" scope=2 deref=0 filter="(mail=li*)" <= bdb_substring_candidates: (mail) not indexed *** glibc detected *** double free or corruption (fasttop): 0x082e54c8 *** Program received signal SIGABRT, Aborted. [Switching to Thread 32771 (LWP 19881)] 0x40350b01 in kill () from /lib/libc.so.6 (gdb) bt #0 0x40350b01 in kill () from /lib/libc.so.6 #1 0x402def65 in pthread_kill () from /lib/libpthread.so.0 #2 0x402defab in raise () from /lib/libpthread.so.0 #3 0x40350894 in raise () from /lib/libc.so.6 #4 0x40351ccc in abort () from /lib/libc.so.6 #5 0x40384b0f in __fsetlocking () from /lib/libc.so.6 #6 0x4038a080 in __libc_malloc_pthread_startup () from /lib/libc.so.6 #7 0x4038b628 in free () from /lib/libc.so.6 #8 0x0813f736 in ber_bvarray_free_x (a=0x82e5470, ctx=0x82882b8) at memory.c:734 #9 0x080a15e3 in slap_set_dispose (cp=<value optimized out>, set=0x82e5470, flags=6) at sets.c:64 #10 0x080a1655 in slap_set_join (cp=0x62f5ab90, lset=0x82e5470, op_flags=16, rset=0x82e5470) at sets.c:335 #11 0x080a23f2 in slap_set_filter (gatherer=0x807f810 <acl_set_gather>, cp=0x62f5ab90, fbv=0x62f5ab98, user=0x82876a0, target=0x82e004c, results=0x0) at sets.c:445 #12 0x0807f5b9 in acl_match_set (subj=0x62f5b380, op=0x8287608, e=0x82e0040, default_set_attribute=0x0) at acl.c:2722 #13 0x08081b58 in access_allowed_mask (op=0x8287608, e=0x82e0040, desc=0x8217f00, val=0x82e40d8, access=ACL_READ, state=0x62f5b428, maskp=0x0) at acl.c:1884 #14 0x0806fa8e in slap_send_search_entry (op=0x8287608, rs=0x6301cce4) at result.c:894 #15 0x080c5eeb in bdb_search (op=0x8287608, rs=0x6301cce4) at search.c:879 #16 0x08062902 in fe_op_search (op=0x8287608, rs=0x6301cce4) at search.c:355 #17 0x080b6ff0 in overlay_op_walk (op=0x8287608, rs=0x6301cce4, which=op_search, oi=0x822fb08, on=0x81eb7d8) at backover.c:650 #18 0x080b73ee in over_op_func (op=0x8287608, rs=0x6301cce4, which=op_search) at backover.c:702 #19 0x0806320b in do_search (op=0x8287608, rs=0x6301cce4) at search.c:217 #20 0x080609db in connection_operation (ctx=0x6301cd58, arg_v=0x8287608) at connection.c:1133 #21 0x0811b664 in ldap_int_thread_pool_wrapper (xpool=0x81d1048) at tpool.c:478 #22 0x402dbc51 in pthread_start_thread () from /lib/libpthread.so.0 #23 0x402dbdb4 in pthread_start_thread_event () from /lib/libpthread.so.0 #24 0x403e238a in clone () from /lib/libc.so.6 8<------------------------------------------------------------------- Jon -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
jclarke@linagora.com a écrit : > Pierangelo Masarati a écrit : >> Should be fixed now in HEAD/re24/re23. Please test. p. > > I've been testing (at last, sorry for the delay), and I've come across > another memory problem. Backtrace is below, and valgrind output is attached. Got this one: it was a double-free in sets.c occuring after a slap_set_join() with lset or rset empty - the non empty set was returned, and then freed, causing a double-free error or segfault. The patch attached corrects this problem on RE23 and HEAD for me and doesn't have any side effects on our test set. However, it may not be the "right" way - please correct if necessary! Your recent fixes have solved all the issues from our test cases we were encountering. Thank you very much for them. Jon -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
jclarke@linagora.com wrote: > Got this one: it was a double-free in sets.c occuring after a > slap_set_join() with lset or rset empty - the non empty set was > returned, and then freed, causing a double-free error or segfault. > > The patch attached corrects this problem on RE23 and HEAD for me and > doesn't have any side effects on our test set. However, it may not be > the "right" way - please correct if necessary! Is your test set something you can clean up for inclusion in our test suite? > Your recent fixes have solved all the issues from our test cases we were > encountering. Thank you very much for them. > > Jon -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
> jclarke@linagora.com a écrit : >> Pierangelo Masarati a écrit : >>> Should be fixed now in HEAD/re24/re23. Please test. p. >> >> I've been testing (at last, sorry for the delay), and I've come across >> another memory problem. Backtrace is below, and valgrind output is >> attached. > > Got this one: it was a double-free in sets.c occuring after a > slap_set_join() with lset or rset empty - the non empty set was > returned, and then freed, causing a double-free error or segfault. > > The patch attached corrects this problem on RE23 and HEAD for me and > doesn't have any side effects on our test set. However, it may not be > the "right" way - please correct if necessary! Thanks for spotting it - I had no time to look at your last report. I'll check the fix and eventually apply it. > Your recent fixes have solved all the issues from our test cases we were > encountering. Thank you very much for them. Sure. Cheers, 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
Howard Chu a écrit : > jclarke@linagora.com wrote: >> Got this one: it was a double-free in sets.c occuring after a >> slap_set_join() with lset or rset empty - the non empty set was >> returned, and then freed, causing a double-free error or segfault. >> >> The patch attached corrects this problem on RE23 and HEAD for me and >> doesn't have any side effects on our test set. However, it may not be >> the "right" way - please correct if necessary! > > Is your test set something you can clean up for inclusion in our test > suite? Unfortunately, it's currently heavily dependant on one of our customer's specific schema extensions and data tree structure. The ACLs in use implement a lot of the logic specific to their environment, therefore I can't send them as is. However, I will see if I can "translate" some of the more complicated ACLs (with sets) to perform on a simple data set for inclusion in the test suite. Could some examples such as these also fit your documentation request for sets and their applications [1]? Assuming this is the case, I will try and provide at least a subset of our test set soon. Cheers, Jon [1] http://www.openldap.org/lists/openldap-software/200709/msg00310.html -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
jclarke@linagora.com wrote: > However, I will see if I can "translate" some of the more complicated > ACLs (with sets) to perform on a simple data set for inclusion in the > test suite. > > Could some examples such as these also fit your documentation request > for sets and their applications [1]? Assuming this is the case, I will > try and provide at least a subset of our test set soon. Absolutely. Thanks... > > Cheers, > Jon > > [1] http://www.openldap.org/lists/openldap-software/200709/msg00310.html -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
> Howard Chu a écrit : > Unfortunately, it's currently heavily dependant on one of our customer's > specific schema extensions and data tree structure. The ACLs in use > implement a lot of the logic specific to their environment, therefore I > can't send them as is. > > However, I will see if I can "translate" some of the more complicated > ACLs (with sets) to perform on a simple data set for inclusion in the > test suite. > > Could some examples such as these also fit your documentation request > for sets and their applications [1]? Assuming this is the case, I will > try and provide at least a subset of our test set soon. > > Cheers, > Jon > > [1] http://www.openldap.org/lists/openldap-software/200709/msg00310.html Let me state first that right now I believe sets do not belong to slapd(8); they should rather move into a module, under the dynacl umbrella. And, as such, they should be documented separately from slapd.access(5), mostly because having them in slapd.access(5) would sort of encourage unexperienced people into (ab-)using them. Also, I'd like sets' users to document them (especially in terms of useful hints, scenarios, case studies and so), as they probably best know what one can gain from their use. Otherwise, a bare description of the syntax, although necessary, would hardly be useful to most of the users. 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
ando@sys-net.it wrote: >> Howard Chu a écrit : > >> Unfortunately, it's currently heavily dependant on one of our customer's >> specific schema extensions and data tree structure. The ACLs in use >> implement a lot of the logic specific to their environment, therefore I >> can't send them as is. >> >> However, I will see if I can "translate" some of the more complicated >> ACLs (with sets) to perform on a simple data set for inclusion in the >> test suite. >> >> Could some examples such as these also fit your documentation request >> for sets and their applications [1]? Assuming this is the case, I will >> try and provide at least a subset of our test set soon. >> >> Cheers, >> Jon >> >> [1] http://www.openldap.org/lists/openldap-software/200709/msg00310.html > > Let me state first that right now I believe sets do not belong to > slapd(8); they should rather move into a module, under the dynacl > umbrella. And, as such, they should be documented separately from > slapd.access(5), mostly because having them in slapd.access(5) would sort > of encourage unexperienced people into (ab-)using them. > > Also, I'd like sets' users to document them (especially in terms of useful > hints, scenarios, case studies and so), as they probably best know what > one can gain from their use. Otherwise, a bare description of the syntax, > although necessary, would hardly be useful to most of the users. > > p. > Agreed. Once decided, we can discuss where they would fit in the Admin guide. -- Kind Regards, Gavin Henry. OpenLDAP Engineering Team. E ghenry@OpenLDAP.org Community developed LDAP software. http://www.openldap.org/project/
jclarke@linagora.com wrote: > Got this one: it was a double-free in sets.c occuring after a > slap_set_join() with lset or rset empty - the non empty set was > returned, and then freed, causing a double-free error or segfault. > > The patch attached corrects this problem on RE23 and HEAD for me and > doesn't have any side effects on our test set. However, it may not be > the "right" way - please correct if necessary! It seems to work correctly, but there seems to be an easier fix: just NULL out the pointer to lset/rset, respectively. I'm patching the code this way, please test if you get a chance. Thanks, 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
changed state Release to Closed
Pierangelo Masarati a écrit : >> The patch attached corrects this problem on RE23 and HEAD for me >> and doesn't have any side effects on our test set. However, it >> may not be the "right" way - please correct if necessary! > > It seems to work correctly, but there seems to be an easier fix: just > NULL out the pointer to lset/rset, respectively. I'm patching the > code this way, please test if you get a chance. Whoops - looks like I spoke too soon. After more testing, I've realized that the same commit this patch applied to actually changed the behaviour of the "+" operation in the slap_set_join() function when operating on empty sets, and can cause errors. The attached proposed patch restores the original design. The difference is minor, but can cause slapd to hang indefinitely on some sets, like the one in the generic example below. I understand an element in a set can have 3 possible states: 1) non-empty string (example: "cn=user,ou=people,dc=example,dc=com") 2) empty string (example: "", BER_BVISEMPTY) 3) NULL (BER_BVISNULL) Below I represent sets in {element1, element2} notation, to describe the current behaviour (in HEAD) for slap_set_join on the '+' operation: - {"One", "Two"} + {"Three", "Four"} ==> {"OneThree", "OneFour", "TwoThree", "TwoFour"} - {"One", "Two"} + {""} ==> {"One", "Two"} - {"One", "Two"} + {NULL} ==> {"One", "Two"} Consider the following set: set="[ldap:///] + [ldap:///<searchbase>??<scope>?<filter1>] + [??<scope>?<filter2>]" Assuming [ldap:///<searchbase>??<scope>?<filter1>] returns a DN, this works. However, if [ldap:///<searchbase>??<scope>?<filter1>] returns no entries, this value is NULL. Therefore, the rest of the set becomes: "[ldap:///] + {NULL} + [??<scope>?<filter2>]" ...which is handed to the '+' operation in slap_set_join, to become: [ldap:///??<scope>?<filter2>] ...which obviously isn't valid. The attached patch changes the last case of the current behaviour for slap_set_join on the '+' operation. This was already the behaviour before version 1.24.2.6 Thu Sep 13 20:43:29 2007 UTC: - {"One", "Two"} + {NULL} ==> {NULL} Thus, in the above example, the whole set would resolve to NULL, and access is not granted. Sorry for the confusion. Can this patch be integrated? Regards, Jonathan -- Jonathan Clarke Cellule OSSA - Groupe LINAGORA 27 rue de Berri, 75008 Paris Tél: 01 58 18 68 28, fax: 01 58 18 68 29 http://www.linagora.com - http://www.08000linux.com
Jonathan Clarke wrote: > Pierangelo Masarati a écrit : >>> The patch attached corrects this problem on RE23 and HEAD for me >>> and doesn't have any side effects on our test set. However, it >>> may not be the "right" way - please correct if necessary! >> It seems to work correctly, but there seems to be an easier fix: just >> NULL out the pointer to lset/rset, respectively. I'm patching the >> code this way, please test if you get a chance. > > Whoops - looks like I spoke too soon. After more testing, I've > realized that the same commit this patch applied to actually changed the > behaviour of the "+" operation in the slap_set_join() function when > operating on empty sets, and can cause errors. > > The attached proposed patch restores the original design. The difference > is minor, but can cause slapd to hang indefinitely on some sets, like > the one in the generic example below. > > I understand an element in a set can have 3 possible states: > 1) non-empty string (example: "cn=user,ou=people,dc=example,dc=com") > 2) empty string (example: "", BER_BVISEMPTY) > 3) NULL (BER_BVISNULL) > > Below I represent sets in {element1, element2} notation, to describe the > current behaviour (in HEAD) for slap_set_join on the '+' operation: > - {"One", "Two"} + {"Three", "Four"} ==> {"OneThree", "OneFour", > "TwoThree", "TwoFour"} > - {"One", "Two"} + {""} ==> {"One", "Two"} > - {"One", "Two"} + {NULL} ==> {"One", "Two"} Although sets lack a specification, I think this behavior is desirable in many cases: adding NULL is equivalent to adding EMPTY (""). > Consider the following set: > set="[ldap:///] + [ldap:///<searchbase>??<scope>?<filter1>] + > [??<scope>?<filter2>]" > > Assuming [ldap:///<searchbase>??<scope>?<filter1>] returns a DN, this > works. However, if [ldap:///<searchbase>??<scope>?<filter1>] returns no > entries, this value is NULL. Therefore, the rest of the set becomes: > "[ldap:///] + {NULL} + [??<scope>?<filter2>]" > ...which is handed to the '+' operation in slap_set_join, to become: > [ldap:///??<scope>?<filter2>] > ...which obviously isn't valid. Well, it is valid: it's an URI with an EMPTY DN. > The attached patch changes the last case of the current behaviour for > slap_set_join on the '+' operation. This was already the behaviour > before version 1.24.2.6 Thu Sep 13 20:43:29 2007 UTC: > - {"One", "Two"} + {NULL} ==> {NULL} This is different from the above behavior, although it makes sense on its own. Probably we need two different addition operators: one inclusive (NULL is equivalent to "") and one exclusive (NULL nullifies the concatenation). Comments? 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 Email: pierangelo.masarati@sys-net.it ---------------------------------------
Hi, Le Sam 10 novembre 2007 16:37, ando@sys-net.it a écrit : >> Below I represent sets in {element1, element2} notation, to describe the >> current behaviour (in HEAD) for slap_set_join on the '+' operation: >> - {"One", "Two"} + {"Three", "Four"} ==> {"OneThree", "OneFour", >> "TwoThree", "TwoFour"} >> - {"One", "Two"} + {""} ==> {"One", "Two"} >> - {"One", "Two"} + {NULL} ==> {"One", "Two"} > > Although sets lack a specification, I think this behavior is desirable > in many cases: adding NULL is equivalent to adding EMPTY (""). The problem is that {NULL} is equivalent to NULL, ie the empty set. >> Consider the following set: >> set="[ldap:///] + [ldap:///<searchbase>??<scope>?<filter1>] + >> [??<scope>?<filter2>]" >> >> Assuming [ldap:///<searchbase>??<scope>?<filter1>] returns a DN, this >> works. However, if [ldap:///<searchbase>??<scope>?<filter1>] returns no >> entries, this value is NULL. Therefore, the rest of the set becomes: >> "[ldap:///] + {NULL} + [??<scope>?<filter2>]" >> ...which is handed to the '+' operation in slap_set_join, to become: >> [ldap:///??<scope>?<filter2>] >> ...which obviously isn't valid. > > Well, it is valid: it's an URI with an EMPTY DN. In terms of LDAP, yes, but it can cause security issues here, as the EMPTY DN is at a higher level that any one which would have been returned by a normal result. >> The attached patch changes the last case of the current behaviour for >> slap_set_join on the '+' operation. This was already the behaviour >> before version 1.24.2.6 Thu Sep 13 20:43:29 2007 UTC: >> - {"One", "Two"} + {NULL} ==> {NULL} > > This is different from the above behavior, although it makes sense on > its own. Probably we need two different addition operators: one > inclusive (NULL is equivalent to "") and one exclusive (NULL nullifies > the concatenation). > > Comments? No problem in having the two operators. I just think the normal behavior is to return NULL, has in term of sets the cartesian product A x NULL returns NULL, not A. Regards, Raphael Ouazana.
moved from Development to Archive.Development
applied to HEAD/re24