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 [email protected] (Thank you, Rich!!)

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);

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;
 }
 

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);
 
 /*