[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: (ITS#8013) SIGSEGV in test_filter()
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--