From 8ecbbf5bfc8ac856d7b593e0f87a6e814c2c64eb Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Thu, 5 Nov 2015 10:44:08 -0800
Subject: [PATCH] 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.
---
 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 4fe7969..8a14431 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 c49f1d1..ff9dd09 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1446,8 +1446,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,
@@ -1488,7 +1487,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

