Full_Name: Leonid Yuriev Version: 2.4-HEAD OS: RHEL7 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (31.130.36.33) I have previously reported on the two SIGSEGV backtraces (see a copy below). This bug was reproduced again during our replication test. I think that now I have found the reason - the PS_FIX_FILTER flag not cleared after update the filter. Patch will be available shortly. Please review and merge. Leonid. === *** Signal 11 (Segmentation fault), address is 0xb from 0x442f07 (0) /opt/openldap.devel/libexec/slapd() [0x442f07]: test_filter /home/ly/Projects/openldap.git/servers/slapd/filterentry.c:69 (1) /opt/openldap.devel/libexec/slapd() [0x514721]: syncprov_matchops /home/ly/Projects/openldap.git/servers/slapd/overlays/syncprov.c:1316 (2) /opt/openldap.devel/libexec/slapd() [0x514b83]: syncprov_op_mod /home/ly/Projects/openldap.git/servers/slapd/overlays/syncprov.c:2145 (3) /opt/openldap.devel/libexec/slapd() [0x48b31a]: overlay_op_walk /home/ly/Projects/openldap.git/servers/slapd/backover.c:662 (4) /opt/openldap.devel/libexec/slapd() [0x48b4c1]: over_op_func /home/ly/Projects/openldap.git/servers/slapd/backover.c:724 (5) /opt/openldap.devel/libexec/slapd() [0x4811a6]: syncrepl_entry /home/ly/Projects/openldap.git/servers/slapd/syncrepl.c:3177 do_syncrep2 /home/ly/Projects/openldap.git/servers/slapd/syncrepl.c:10%0 (6) /opt/openldap.devel/libexec/slapd() [0x4844b2]: do_syncrepl /home/ly/Projects/openldap.git/servers/slapd/syncrepl.c:1539 === Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f9762ffe700 (LWP 29507)]D%D test_filter (op=0x7f9762ffc210, e=0x7f96f19c37d8, f=0x20) at filterentry.c:69 69 if ( f->f_choice & SLAPD_FILTER_UNDEFINED ) { (0) /opt/openldap.devel/libexec/slapd() [0x4430b7]: test_filter /home/ly/Projects/openldap.git/servers/slapd/filterentry.c:69 (1) /opt/openldap.devel/libexec/slapd() [0x515081]: syncprov_matchops /home/ly/Projects/openldap.git/servers/slapd/overlays/syncprov.c:1317 (2) /opt/openldap.devel/libexec/slapd() [0x515f43]: syncprov_op_response /home/ly/Projects/openldap.git/servers/slapd/overlays/syncprov.c:1941 (3) /opt/openldap.devel/libexec/slapd() [0x434163]: slap_response_play /home/ly/Projects/openldap.git/servers/slapd/result.c:509 (4) /opt/openldap.devel/libexec/slapd() [0x4346ca]: send_ldap_response /home/ly/Projects/openldap.git/servers/slapd/result.c:584 (5) /opt/openldap.devel/libexec/slapd() [0x435062]: slap_send_ldap_result /home/ly/Projects/openldap.git/servers/slapd/result.c:861 (6) /opt/openldap.devel/libexec/slapd() [0x4cb2e9]: mdb_add /home/ly/Projects/openldap.git/servers/slapd/back-mdb/add.c:434 (7) /opt/openldap.devel/libexec/slapd() [0x48b506]: overlay_op_walk /home/ly/Projects/openldap.git/servers/slapd/backover.c:674 (8) /opt/openldap.devel/libexec/slapd() [0x48b671]: over_op_func /home/ly%Projojects/openldap.git/servers/slapd/backover.c:724
Please review attached patch and merge. Leonid. --- The attached files is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights and/or interest in this work to any party. I, Leonid Yuriev am authorized by Peter-Service LLC, my employer, to release this work under the following terms. Peter-Service LLC hereby places the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice.
Leonid Yuriev wrote: > Please review attached patch and merge. The patch is wrong. syncprov_matchops can be called twice for the same operation; toggling the flag where you've placed it means the filter will be incorrect on the 2nd invocation as well as on all subsequent invocations. > > Leonid. > > --- > > The attached files is derived from OpenLDAP Software. All of the > modifications > to OpenLDAP Software represented in the following patch(es) were > developed by > Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned > rights > and/or interest in this work to any party. I, Leonid Yuriev am > authorized by > Peter-Service LLC, my employer, to release this work under the following > terms. > > Peter-Service LLC hereby places the following modifications to OpenLDAP > Software > (and only these modifications) into the public domain. Hence, these > modifications may be freely used and/or redistributed for any purpose > with or without attribution and/or other notice. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
04.01.2015 02:47, Howard Chu wrote: > The patch is wrong. syncprov_matchops can be called twice for the same > operation; toggling the flag where you've placed it means the filter > will be incorrect on the 2nd invocation as well as on all subsequent > invocations. Ok, but found another case. The ss->s_op->ors_filter may be updated immediately after the s_mutex released. This is a simple change for reproducing the bug. diff --git a/servers/slapd/overlays/syncprov.c b/servers/slapd/overlays/syncprov.c index 5ef2eaa..f579faa 100644 --- a/servers/slapd/overlays/syncprov.c +++ b/servers/slapd/overlays/syncprov.c @@ -1295,6 +1295,7 @@ syncprov_matchops( Operation *op, opcookie *opc, int saveit ) } if ( fc.fscope ) { + Filter *paranoia; ldap_pvt_thread_mutex_lock( &ss->s_mutex ); op2 = *ss->s_op; oh = *op->o_hdr; @@ -1304,6 +1305,7 @@ syncprov_matchops( Operation *op, opcookie *opc, int saveit ) op2.o_hdr = &oh; op2.o_extra = op->o_extra; op2.o_callback = NULL; + paranoia = ss->s_op->ors_filter; if (ss->s_flags & PS_FIX_FILTER) { /* Skip the AND/GE clause that we stuck on in front. We would lose deletes/mods that happen during the refresh @@ -1311,7 +1313,11 @@ syncprov_matchops( Operation *op, opcookie *opc, int saveit ) op2.ors_filter = ss->s_op->ors_filter->f_and->f_next; } ldap_pvt_thread_mutex_unlock( &ss->s_mutex ); + assert (paranoia == ss->s_op->ors_filter || paranoia == ss->s_op->ors_filter->f_and->f_next); + usleep(1000); + assert (paranoia == ss->s_op->ors_filter || paranoia == ss->s_op->ors_filter->f_and->f_next); rc = test_filter( &op2, e, op2.ors_filter ); + assert (paranoia == ss->s_op->ors_filter || paranoia == ss->s_op->ors_filter->f_and->f_next); } Debug( LDAP_DEBUG_TRACE, "syncprov_matchops: sid %03x fscope %d rc %d\n", Updated patch attached. Please review and merge. Leonid. --- The attached files is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights and/or interest in this work to any party. I, Leonid Yuriev am authorized by Peter-Service LLC, my employer, to release this work under the following terms. Peter-Service LLC hereby places the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice.
Leonid Yuriev wrote: > > 04.01.2015 02:47, Howard Chu wrote: >> The patch is wrong. syncprov_matchops can be called twice for the same >> operation; toggling the flag where you've placed it means the filter >> will be incorrect on the 2nd invocation as well as on all subsequent >> invocations. > > Ok, but found another case. > The ss->s_op->ors_filter may be updated immediately after the s_mutex > released. Seems like the simpler fix then is to keep the mutex held until after test_filter finishes. > > This is a simple change for reproducing the bug. > > diff --git a/servers/slapd/overlays/syncprov.c > b/servers/slapd/overlays/syncprov.c > index 5ef2eaa..f579faa 100644 > --- a/servers/slapd/overlays/syncprov.c > +++ b/servers/slapd/overlays/syncprov.c > @@ -1295,6 +1295,7 @@ syncprov_matchops( Operation *op, opcookie *opc, > int saveit ) > } > > if ( fc.fscope ) { > + Filter *paranoia; > ldap_pvt_thread_mutex_lock( &ss->s_mutex ); > op2 = *ss->s_op; > oh = *op->o_hdr; > @@ -1304,6 +1305,7 @@ syncprov_matchops( Operation *op, opcookie *opc, > int saveit ) > op2.o_hdr = &oh; > op2.o_extra = op->o_extra; > op2.o_callback = NULL; > + paranoia = ss->s_op->ors_filter; > if (ss->s_flags & PS_FIX_FILTER) { > /* Skip the AND/GE clause that we stuck on in front. We > would lose deletes/mods that happen during the refresh > @@ -1311,7 +1313,11 @@ syncprov_matchops( Operation *op, opcookie *opc, > int saveit ) > op2.ors_filter = ss->s_op->ors_filter->f_and->f_next; > } > ldap_pvt_thread_mutex_unlock( &ss->s_mutex ); > + assert (paranoia == ss->s_op->ors_filter || paranoia == > ss->s_op->ors_filter->f_and->f_next); > + usleep(1000); > + assert (paranoia == ss->s_op->ors_filter || paranoia == > ss->s_op->ors_filter->f_and->f_next); > rc = test_filter( &op2, e, op2.ors_filter ); > + assert (paranoia == ss->s_op->ors_filter || paranoia == > ss->s_op->ors_filter->f_and->f_next); > } > > Debug( LDAP_DEBUG_TRACE, "syncprov_matchops: sid %03x fscope > %d rc %d\n", > > Updated patch attached. > Please review and merge. > > Leonid. > > --- > > The attached files is derived from OpenLDAP Software. All of the > modifications > to OpenLDAP Software represented in the following patch(es) were > developed by > Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned > rights > and/or interest in this work to any party. I, Leonid Yuriev am > authorized by > Peter-Service LLC, my employer, to release this work under the following > terms. > > Peter-Service LLC hereby places the following modifications to OpenLDAP > Software > (and only these modifications) into the public domain. Hence, these > modifications may be freely used and/or redistributed for any purpose > with or without attribution and/or other notice. > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
hyc@symas.com wrote: > Leonid Yuriev wrote: >> >> 04.01.2015 02:47, Howard Chu wrote: >>> The patch is wrong. syncprov_matchops can be called twice for the same >>> operation; toggling the flag where you've placed it means the filter >>> will be incorrect on the 2nd invocation as well as on all subsequent >>> invocations. >> >> Ok, but found another case. >> The ss->s_op->ors_filter may be updated immediately after the s_mutex >> released. > > Seems like the simpler fix then is to keep the mutex held until after > test_filter finishes. Fixed in master. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
changed notes changed state Open to Test moved from Incoming to Software Bugs
changed notes changed state Test to Release
fixed in master fixed in RE25 fixed in RE24
changed notes changed state Release to Closed