[389-commits] 3 commits - ldap/servers
Noriko Hosoi
nhosoi at fedoraproject.org
Sun May 10 04:01:53 UTC 2015
ldap/servers/slapd/opshared.c | 93 ++++++++++++++++++--------------------
ldap/servers/slapd/pagedresults.c | 11 ++++
ldap/servers/slapd/proto-slap.h | 2
ldap/servers/slapd/result.c | 32 ++++++++++++-
4 files changed, 88 insertions(+), 50 deletions(-)
New commits:
commit 4051fe0eba4128bae88fecfbf2c9441aeb334796
Author: Noriko Hosoi <nhosoi at redhat.com>
Date: Mon May 4 14:06:43 2015 -0700
Ticket #48146 - async simple paged results issue
Description: Invalid index could cause Invalid read.
https://fedorahosted.org/389/ticket/48146
(cherry picked from commit 8e21bfbe4fcac79cf39e5c6b579c4bc88e05257e)
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index a3a5fc4..327da54 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -138,6 +138,13 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
memcpy(ptr, cookie.bv_val, cookie.bv_len);
*(ptr+cookie.bv_len) = '\0';
*index = strtol(ptr, NULL, 10);
+ if (conn->c_pagedresults.prl_maxlen <= *index) {
+ rc = LDAP_PROTOCOL_ERROR;
+ LDAPDebug1Arg(LDAP_DEBUG_ANY,
+ "pagedresults_parse_control_value: invalid cookie: %d\n",
+ *index);
+ goto bail;
+ }
slapi_ch_free_string(&ptr);
prp = conn->c_pagedresults.prl_list + *index;
if (!(prp->pr_search_result_set)) { /* freed and reused for the next backend. */
@@ -162,6 +169,7 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
"pagedresults_parse_control_value: invalid cookie: %d\n",
*index);
}
+bail:
PR_Unlock(conn->c_mutex);
LDAPDebug1Arg(LDAP_DEBUG_TRACE,
commit 5e9c4f1f09596dd076a6c3a0bb419580e8fd705e
Author: Noriko Hosoi <nhosoi at redhat.com>
Date: Fri May 1 12:01:13 2015 -0700
Ticket #48146 - async simple paged results issue; need to close a small window for a pr index competed among multiple threads.
Description: If multiple async simple paged results requests come in via one
connection simultaneously, the same slot in the paged results array in the
connection could be shared. If one of them has to do paging, the search
request object stashed in the paged result array slot could be freed by the
other request if it has the shorter life cycle.
These 3 reqs use the same paged results array slot.
req0: <--------------><----x
page1 page2
req1: <----->
req2: <------->
frees search result object of req0
Reviewed by rmeggins at redhat.com (Thank you, Rich!!)
(cherry picked from commit 3cf85d1ad6cbc0feac5578dee5ce259c0f65055f)
diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index 272e550..1d454b7 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -408,6 +408,51 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
*/
operation_set_target_spec (pb->pb_op, basesdn);
+ if (be_name == NULL)
+ {
+ /* no specific backend was requested, use the mapping tree
+ */
+ err_code = slapi_mapping_tree_select_all(pb, be_list, referral_list, errorbuf);
+ if (((err_code != LDAP_SUCCESS) && (err_code != LDAP_OPERATIONS_ERROR) && (err_code != LDAP_REFERRAL))
+ || ((err_code == LDAP_OPERATIONS_ERROR) && (be_list[0] == NULL)))
+ {
+ send_ldap_result(pb, err_code, NULL, errorbuf, 0, NULL);
+ rc = -1;
+ goto free_and_return;
+ }
+ if (be_list[0] != NULL)
+ {
+ index = 0;
+ if (pr_be) { /* PAGED RESULT: be is found from the previous paging. */
+ /* move the index in the be_list which matches pr_be */
+ while (be_list[index] && be_list[index+1] && pr_be != be_list[index])
+ index++;
+ } else {
+ while (be_list[index] && be_list[index+1])
+ index++;
+ }
+ /* "be" is either pr_be or the last backend */
+ be = be_list[index];
+ }
+ else
+ be = pr_be?pr_be:NULL;
+ }
+ else
+ {
+ /* specific backend be_name was requested, use slapi_be_select_by_instance_name
+ */
+ if (pr_be) {
+ be_single = be = pr_be;
+ } else {
+ be_single = be = slapi_be_select_by_instance_name(be_name);
+ }
+ if (be_single)
+ slapi_be_Rlock(be_single);
+ be_list[0] = NULL;
+ referral_list[0] = NULL;
+ referral = NULL;
+ }
+
/* this is time to check if mapping tree specific control
* was used to specify that we want to parse only
* one backend
@@ -478,8 +523,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
if ( slapi_control_present (ctrlp, LDAP_CONTROL_PAGEDRESULTS,
&ctl_value, &iscritical) )
{
- rc = pagedresults_parse_control_value(pb, ctl_value,
- &pagesize, &pr_idx);
+ rc = pagedresults_parse_control_value(pb, ctl_value, &pagesize, &pr_idx, be);
/* Let's set pr_idx even if it fails; in case, pr_idx == -1. */
slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
if ((LDAP_SUCCESS == rc) ||
@@ -520,51 +564,6 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
}
}
- if (be_name == NULL)
- {
- /* no specific backend was requested, use the mapping tree
- */
- err_code = slapi_mapping_tree_select_all(pb, be_list, referral_list, errorbuf);
- if (((err_code != LDAP_SUCCESS) && (err_code != LDAP_OPERATIONS_ERROR) && (err_code != LDAP_REFERRAL))
- || ((err_code == LDAP_OPERATIONS_ERROR) && (be_list[0] == NULL)))
- {
- send_ldap_result(pb, err_code, NULL, errorbuf, 0, NULL);
- rc = -1;
- goto free_and_return;
- }
- if (be_list[0] != NULL)
- {
- index = 0;
- if (pr_be) { /* PAGED RESULT: be is found from the previous paging. */
- /* move the index in the be_list which matches pr_be */
- while (be_list[index] && be_list[index+1] && pr_be != be_list[index])
- index++;
- } else {
- while (be_list[index] && be_list[index+1])
- index++;
- }
- /* "be" is either pr_be or the last backend */
- be = be_list[index];
- }
- else
- be = pr_be?pr_be:NULL;
- }
- else
- {
- /* specific backend be_name was requested, use slapi_be_select_by_instance_name
- */
- if (pr_be) {
- be_single = be = pr_be;
- } else {
- be_single = be = slapi_be_select_by_instance_name(be_name);
- }
- if (be_single)
- slapi_be_Rlock(be_single);
- be_list[0] = NULL;
- referral_list[0] = NULL;
- referral = NULL;
- }
-
slapi_pblock_set(pb, SLAPI_BACKEND_COUNT, &index);
if (be)
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index e61c000..a3a5fc4 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -58,7 +58,7 @@
int
pagedresults_parse_control_value( Slapi_PBlock *pb,
struct berval *psbvp, ber_int_t *pagesize,
- int *index )
+ int *index, Slapi_Backend *be )
{
int rc = LDAP_SUCCESS;
struct berval cookie = {0};
@@ -119,6 +119,7 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
} else {
for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
if (!conn->c_pagedresults.prl_list[i].pr_current_be) {
+ conn->c_pagedresults.prl_list[i].pr_current_be = be;
*index = i;
break;
}
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 408543e..ee32bd0 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1487,7 +1487,7 @@ int slapd_do_all_nss_ssl_init(int slapd_exemode, int importexport_encrypt,
* pagedresults.c
*/
int pagedresults_parse_control_value(Slapi_PBlock *pb, struct berval *psbvp,
- ber_int_t *pagesize, int *index);
+ ber_int_t *pagesize, int *index, Slapi_Backend *be);
void pagedresults_set_response_control(Slapi_PBlock *pb, int iscritical,
ber_int_t estimate,
int curr_search_count, int index);
commit 09c730bec94bfd5f40f46da467a93768ced061f6
Author: Noriko Hosoi <nhosoi at redhat.com>
Date: Fri May 1 10:50:39 2015 -0700
Ticket #48146 - async simple paged results issue; log pr index
Description: When the request is a simple paged results search,
log pr index in the access log.
Sample access log for "getent netgroup <name>":
[..] conn=23 op=521 SRCH base="cn=accounts,dc=testrelm,dc=test" scope=2 filter="(&(|(memberOf=ipaUniqueID=3036b6a0-ee18-11e4-b7dd-00215e2032c0,cn=ng,cn=alt,dc=testrelm,dc=test))(objectClass=ipaHost))" attrs="objectClass cn fqdn serverHostName memberOf ipaSshPubKey ipaUniqueID"
[..] conn=23 op=521 RESULT err=0 tag=101 nentries=200 etime=0 notes=P pr_idx=3
https://fedorahosted.org/389/ticket/48146
Reviewed by rmeggins at redhat.com (Thanks, Rich!!)
(cherry picked from commit 9eb20d89755160aa916544c7cfce0ad066e538f7)
diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c
index d5984df..6ff70e3 100644
--- a/ldap/servers/slapd/result.c
+++ b/ldap/servers/slapd/result.c
@@ -1961,6 +1961,9 @@ log_result( Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentrie
CSN *operationcsn = NULL;
char csn_str[CSN_STRSIZE + 5];
char etime[ETIME_BUFSIZ];
+ int pr_idx = -1;
+
+ slapi_pblock_get(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
internal_op = operation_is_flag_set( op, OP_FLAG_INTERNAL );
@@ -2054,7 +2057,34 @@ log_result( Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentrie
}
slapi_ch_free((void**)&dn);
} else {
- if ( !internal_op )
+ if (pr_idx > -1)
+ {
+ if ( !internal_op )
+ {
+ slapi_log_access( LDAP_DEBUG_STATS,
+ "conn=%" NSPRIu64 " op=%d RESULT err=%d"
+ " tag=%" BERTAG_T " nentries=%d etime=%s%s%s"
+ " pr_idx=%d\n",
+ op->o_connid,
+ op->o_opid,
+ err, tag, nentries,
+ etime,
+ notes_str, csn_str, pr_idx );
+ }
+ else
+ {
+ slapi_log_access( LDAP_DEBUG_ARGS,
+ "conn=%s op=%d RESULT err=%d"
+ " tag=%" BERTAG_T " nentries=%d etime=%s%s%s"
+ " pr_idx=%d\n",
+ LOG_INTERNAL_OP_CON_ID,
+ LOG_INTERNAL_OP_OP_ID,
+ err, tag, nentries,
+ etime,
+ notes_str, csn_str, pr_idx );
+ }
+ }
+ else if ( !internal_op )
{
slapi_log_access( LDAP_DEBUG_STATS,
"conn=%" NSPRIu64 " op=%d RESULT err=%d"
More information about the 389-commits
mailing list