From 24129b41820b87d613e721f8530e1955f1cce0ff Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Thu, 5 Nov 2015 10:44:08 -0800 Subject: [PATCH 72/75] Ticket #48338 - SimplePagedResults -- abandon could happen between the abandon check and sending results Description: An abandon request for a SimplePagedResults request could happened between the abandon check and the code for sending the search results. The abandon frees the search results although sending result code still refers it. Fix description: The code (from getting search results through sending the search results) in op_shared_search is protected by c_mutex locking. https://fedorahosted.org/389/ticket/48338 Reviewed by rmeggins@redhat.com (Thank you, Rich!!) (cherry picked from commit 390b8bd9076e8976facc0858e60985d6b4fac05c) (cherry picked from commit 8f49d33d30fade7b579062414250a0ddb1a66c62) --- ldap/servers/slapd/opshared.c | 13 ++++++++----- ldap/servers/slapd/pagedresults.c | 28 ++++++++++++++++++---------- ldap/servers/slapd/proto-slap.h | 5 ++--- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index dcdbb04..586ca1f 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -500,7 +500,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result) be = be_list[index]; } } - pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, pr_idx); + pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, 0/*not locked*/, pr_idx); estimate = pagedresults_get_search_result_set_size_estimate(pb->pb_conn, operation, pr_idx); if (pagedresults_get_unindexed(pb->pb_conn, operation, pr_idx)) { opnote |= SLAPI_OP_NOTE_UNINDEXED; @@ -675,13 +675,15 @@ op_shared_search (Slapi_PBlock *pb, int send_result) * In async paged result case, the search result might be released * by other theads. We need to double check it in the locked region. */ - pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, pr_idx); + PR_Lock(pb->pb_conn->c_mutex); + pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, 1/*locked*/, pr_idx); if (pr_search_result) { - if (pagedresults_is_abandoned_or_notavailable(pb->pb_conn, pr_idx)) { + if (pagedresults_is_abandoned_or_notavailable(pb->pb_conn, 1/*locked*/, pr_idx)) { pagedresults_unlock(pb->pb_conn, pr_idx); /* Previous operation was abandoned and the simplepaged object is not in use. */ send_ldap_result(pb, 0, NULL, "Simple Paged Results Search abandoned", 0, NULL); rc = LDAP_SUCCESS; + PR_Unlock(pb->pb_conn->c_mutex); goto free_and_return; } else { slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, pr_search_result ); @@ -689,7 +691,8 @@ op_shared_search (Slapi_PBlock *pb, int send_result) /* search result could be reset in the backend/dse */ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); - pagedresults_set_search_result(pb->pb_conn, operation, sr, 0, pr_idx); + pagedresults_set_search_result(pb->pb_conn, operation, sr, 1/*locked*/, pr_idx); + PR_Unlock(pb->pb_conn->c_mutex); } } else { pr_stat = PAGEDRESULTS_SEARCH_END; @@ -720,7 +723,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result) if (PAGEDRESULTS_SEARCH_END == pr_stat) { pagedresults_lock(pb->pb_conn, pr_idx); slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET, NULL); - if (!pagedresults_is_abandoned_or_notavailable(pb->pb_conn, pr_idx)) { + if (!pagedresults_is_abandoned_or_notavailable(pb->pb_conn, 0/*not locked*/, pr_idx)) { pagedresults_free_one(pb->pb_conn, operation, pr_idx); } pagedresults_unlock(pb->pb_conn, pr_idx); diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c index 87447c4..4458cfb 100644 --- a/ldap/servers/slapd/pagedresults.c +++ b/ldap/servers/slapd/pagedresults.c @@ -395,20 +395,25 @@ pagedresults_set_current_be(Connection *conn, Slapi_Backend *be, int index, int } void * -pagedresults_get_search_result(Connection *conn, Operation *op, int index) +pagedresults_get_search_result(Connection *conn, Operation *op, int locked, int index) { void *sr = NULL; if (!op_is_pagedresults(op)) { return sr; /* noop */ } - LDAPDebug1Arg(LDAP_DEBUG_TRACE, - "--> pagedresults_get_search_result: idx=%d\n", index); + LDAPDebug2Args(LDAP_DEBUG_TRACE, + "--> pagedresults_get_search_result(%s): idx=%d\n", + locked?"locked":"not locked", index); if (conn && (index > -1)) { - PR_Lock(conn->c_mutex); + if (!locked) { + PR_Lock(conn->c_mutex); + } if (index < conn->c_pagedresults.prl_maxlen) { sr = conn->c_pagedresults.prl_list[index].pr_search_result_set; } - PR_Unlock(conn->c_mutex); + if (!locked) { + PR_Unlock(conn->c_mutex); + } } LDAPDebug1Arg(LDAP_DEBUG_TRACE, "<-- pagedresults_get_search_result: %p\n", sr); @@ -416,8 +421,7 @@ pagedresults_get_search_result(Connection *conn, Operation *op, int index) } int -pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, - int locked, int index) +pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, int locked, int index) { int rc = -1; if (!op_is_pagedresults(op)) { @@ -1003,15 +1007,19 @@ pagedresults_unlock( Connection *conn, int index ) } int -pagedresults_is_abandoned_or_notavailable( Connection *conn, int index ) +pagedresults_is_abandoned_or_notavailable(Connection *conn, int locked, int index) { PagedResults *prp; if (!conn || (index < 0) || (index >= conn->c_pagedresults.prl_maxlen)) { return 1; /* not abandoned, but do not want to proceed paged results op. */ } - PR_Lock(conn->c_mutex); + if (!locked) { + PR_Lock(conn->c_mutex); + } prp = conn->c_pagedresults.prl_list + index; - PR_Unlock(conn->c_mutex); + if (!locked) { + PR_Unlock(conn->c_mutex); + } return prp->pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED; } diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index b10c1eb..e1cb53e 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -1445,8 +1445,7 @@ void pagedresults_set_response_control(Slapi_PBlock *pb, int iscritical, int curr_search_count, int index); Slapi_Backend *pagedresults_get_current_be(Connection *conn, int index); int pagedresults_set_current_be(Connection *conn, Slapi_Backend *be, int index, int nolock); -void *pagedresults_get_search_result(Connection *conn, Operation *op, - int index); +void *pagedresults_get_search_result(Connection *conn, Operation *op, int locked, int index); int pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, int locked, int index); int pagedresults_get_search_result_count(Connection *conn, Operation *op, @@ -1487,7 +1486,7 @@ int pagedresults_cleanup_all(Connection *conn, int needlock); void op_set_pagedresults(Operation *op); void pagedresults_lock(Connection *conn, int index); void pagedresults_unlock(Connection *conn, int index); -int pagedresults_is_abandoned_or_notavailable(Connection *conn, int index); +int pagedresults_is_abandoned_or_notavailable(Connection *conn, int locked, int index); int pagedresults_set_search_result_pb(Slapi_PBlock *pb, void *sr, int locked); /* -- 2.4.3