This is a multi-part message in MIME format. --------------070009030106040804000907 Content-Type: text/plain; charset=windows-1251; format=flowed Content-Transfer-Encoding: 7bit
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.
--------------070009030106040804000907 Content-Type: text/x-patch; name="its8013.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="its8013.patch"
commit 5014e0b526e68c21bcfd9a9dd43d154d621f7476 Author: Leo Yuriev leo@yuriev.ru Date: 2015-01-01 16:44:50 +0300
ITS#8013 fix rare SIGSEGV in test_filter().
Filter may be updated/fixed and freed in other thread immediately after release the s_mutex, while the test_filter() running.
Also now specifically damaged filter fields when it freed. This will enable faster detection of such errors (fail-fast by SIGSEGV).
diff --git a/servers/slapd/filter.c b/servers/slapd/filter.c index b859f73..f440336 100644 --- a/servers/slapd/filter.c +++ b/servers/slapd/filter.c @@ -566,6 +566,8 @@ filter_free_x( Operation *op, Filter *f, int freeme ) break; }
+ f->f_next = (void*) -1l; + f->f_un.f_un_complex = (void*) -1l; if ( freeme ) { op->o_tmpfree( f, op->o_tmpmemctx ); } diff --git a/servers/slapd/overlays/syncprov.c b/servers/slapd/overlays/syncprov.c index 5ef2eaa..2369edb 100644 --- a/servers/slapd/overlays/syncprov.c +++ b/servers/slapd/overlays/syncprov.c @@ -1304,14 +1304,17 @@ syncprov_matchops( Operation *op, opcookie *opc, int saveit ) op2.o_hdr = &oh; op2.o_extra = op->o_extra; op2.o_callback = NULL; - if (ss->s_flags & PS_FIX_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 phase otherwise (ITS#6555) */ - op2.ors_filter = ss->s_op->ors_filter->f_and->f_next; + op2.ors_filter = filter_dup( op2.ors_filter->f_and->f_next, NULL ); + } else { + op2.ors_filter = filter_dup( op2.ors_filter, NULL ); } ldap_pvt_thread_mutex_unlock( &ss->s_mutex ); rc = test_filter( &op2, e, op2.ors_filter ); + filter_free( op2.ors_filter ); }
Debug( LDAP_DEBUG_TRACE, "syncprov_matchops: sid %03x fscope %d rc %d\n", @@ -2237,6 +2240,7 @@ syncprov_detach_op( Operation *op, syncops *so, slap_overinst *on )
/* Skip the AND/GE clause that we stuck on in front */ if ( so->s_flags & PS_FIX_FILTER ) { + assert(op2->ors_filter->f_choice == LDAP_FILTER_AND); op2->ors_filter = op->ors_filter->f_and->f_next; so->s_flags ^= PS_FIX_FILTER; } else { @@ -2390,7 +2394,6 @@ syncprov_search_response( Operation *op, SlapReply *rs ) ldap_pvt_thread_mutex_unlock( &op->o_conn->c_mutex ); /* syncprov_ab_cleanup will free this syncop */ return SLAPD_ABANDON; - } else { ldap_pvt_thread_mutex_lock( &ss->ss_so->s_mutex ); /* Turn off the refreshing flag */
--------------070009030106040804000907--