diff --git a/SOURCES/0021-Issue-6509-Race-condition-with-Paged-Result-searches.patch b/SOURCES/0021-Issue-6509-Race-condition-with-Paged-Result-searches.patch new file mode 100644 index 0000000..c15fd7e --- /dev/null +++ b/SOURCES/0021-Issue-6509-Race-condition-with-Paged-Result-searches.patch @@ -0,0 +1,297 @@ +From d2f9dd82e3610ee9b73feea981c680c03bb21394 Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Thu, 16 Jan 2025 08:42:53 -0500 +Subject: [PATCH] Issue 6509 - Race condition with Paged Result searches + +Description: + +There is a race condition with Paged Result searches when a new operation comes +in while a paged search is finishing. This triggers an invalid time out error +and closes the connection with a T3 code. + +The problem is that we do not use the "PagedResult lock" when checking the +connection's paged result data for a timeout event. This causes the paged +result timeout value to change unexpectedly and trigger a false timeout when a +new operation arrives. + +Now we check the timeout without hte conn lock, if its expired it could +be a race condition and false positive. Try the lock again and test the +timeout. This also prevents blocking non-paged result searches from +getting held up by the lock when it's not necessary. + +This also fixes some memory leaks that occur when an error happens. + +Relates: https://github.com/389ds/389-ds-base/issues/6509 + +Reviewed by: tbordaz & proger (Thanks!!) +--- + ldap/servers/slapd/daemon.c | 61 ++++++++++++++++++------------- + ldap/servers/slapd/opshared.c | 58 ++++++++++++++--------------- + ldap/servers/slapd/pagedresults.c | 9 +++++ + ldap/servers/slapd/slap.h | 2 +- + 4 files changed, 75 insertions(+), 55 deletions(-) + +diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c +index bb80dae36..13dfe250d 100644 +--- a/ldap/servers/slapd/daemon.c ++++ b/ldap/servers/slapd/daemon.c +@@ -1578,7 +1578,29 @@ setup_pr_read_pds(Connection_Table *ct) + if (c->c_state == CONN_STATE_FREE) { + connection_table_move_connection_out_of_active_list(ct, c); + } else { +- /* we try to acquire the connection mutex, if it is already ++ /* Check for a timeout for PAGED RESULTS */ ++ if (pagedresults_is_timedout_nolock(c)) { ++ /* ++ * There could be a race condition so lets try again with the ++ * right lock ++ */ ++ pthread_mutex_t *pr_mutex = pageresult_lock_get_addr(c); ++ if (pthread_mutex_trylock(pr_mutex) == EBUSY) { ++ c = next; ++ continue; ++ } ++ if (pagedresults_is_timedout_nolock(c)) { ++ pthread_mutex_unlock(pr_mutex); ++ disconnect_server(c, c->c_connid, -1, ++ SLAPD_DISCONNECT_PAGED_SEARCH_LIMIT, ++ 0); ++ } else { ++ pthread_mutex_unlock(pr_mutex); ++ } ++ } ++ ++ /* ++ * we try to acquire the connection mutex, if it is already + * acquired by another thread, don't wait + */ + if (pthread_mutex_trylock(&(c->c_mutex)) == EBUSY) { +@@ -1586,35 +1608,24 @@ setup_pr_read_pds(Connection_Table *ct) + continue; + } + if (c->c_flags & CONN_FLAG_CLOSING) { +- /* A worker thread has marked that this connection +- * should be closed by calling disconnect_server. +- * move this connection out of the active list +- * the last thread to use the connection will close it ++ /* ++ * A worker thread, or paged result timeout, has marked that ++ * this connection should be closed by calling ++ * disconnect_server(). Move this connection out of the active ++ * list then the last thread to use the connection will close ++ * it. + */ + connection_table_move_connection_out_of_active_list(ct, c); + } else if (c->c_sd == SLAPD_INVALID_SOCKET) { + connection_table_move_connection_out_of_active_list(ct, c); + } else if (c->c_prfd != NULL) { + if ((!c->c_gettingber) && (c->c_threadnumber < c->c_max_threads_per_conn)) { +- int add_fd = 1; +- /* check timeout for PAGED RESULTS */ +- if (pagedresults_is_timedout_nolock(c)) { +- /* Exceeded the paged search timelimit; disconnect the client */ +- disconnect_server_nomutex(c, c->c_connid, -1, +- SLAPD_DISCONNECT_PAGED_SEARCH_LIMIT, +- 0); +- connection_table_move_connection_out_of_active_list(ct, +- c); +- add_fd = 0; /* do not poll on this fd */ +- } +- if (add_fd) { +- ct->fd[count].fd = c->c_prfd; +- ct->fd[count].in_flags = SLAPD_POLL_FLAGS; +- /* slot i of the connection table is mapped to slot +- * count of the fds array */ +- c->c_fdi = count; +- count++; +- } ++ ct->fd[listnum][count].fd = c->c_prfd; ++ ct->fd[listnum][count].in_flags = SLAPD_POLL_FLAGS; ++ /* slot i of the connection table is mapped to slot ++ * count of the fds array */ ++ c->c_fdi = count; ++ count++; + } else { + if (c->c_threadnumber >= c->c_max_threads_per_conn) { + c->c_maxthreadsblocked++; +@@ -1675,7 +1686,7 @@ handle_pr_read_ready(Connection_Table *ct, PRIntn num_poll __attribute__((unused + continue; + } + +- /* Try to get connection mutex, if not available just skip the connection and ++ /* Try to get connection mutex, if not available just skip the connection and + * process other connections events. May generates cpu load for listening thread + * if connection mutex is held for a long time + */ +diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c +index 7ab4117cd..a29eed052 100644 +--- a/ldap/servers/slapd/opshared.c ++++ b/ldap/servers/slapd/opshared.c +@@ -250,7 +250,7 @@ op_shared_search(Slapi_PBlock *pb, int send_result) + char *errtext = NULL; + int nentries, pnentries; + int flag_search_base_found = 0; +- int flag_no_such_object = 0; ++ bool flag_no_such_object = false; + int flag_referral = 0; + int flag_psearch = 0; + int err_code = LDAP_SUCCESS; +@@ -315,7 +315,7 @@ op_shared_search(Slapi_PBlock *pb, int send_result) + rc = -1; + goto free_and_return_nolock; + } +- ++ + /* Set the time we actually started the operation */ + slapi_operation_set_time_started(operation); + +@@ -798,11 +798,11 @@ op_shared_search(Slapi_PBlock *pb, int send_result) + } + + /* subtree searches : +- * if the search was started above the backend suffix +- * - temporarily set the SLAPI_SEARCH_TARGET_SDN to the +- * base of the node so that we don't get a NO SUCH OBJECT error +- * - do not change the scope +- */ ++ * if the search was started above the backend suffix ++ * - temporarily set the SLAPI_SEARCH_TARGET_SDN to the ++ * base of the node so that we don't get a NO SUCH OBJECT error ++ * - do not change the scope ++ */ + if (scope == LDAP_SCOPE_SUBTREE) { + if (slapi_sdn_issuffix(be_suffix, basesdn)) { + if (free_sdn) { +@@ -825,53 +825,53 @@ op_shared_search(Slapi_PBlock *pb, int send_result) + switch (rc) { + case 1: + /* if the backend returned LDAP_NO_SUCH_OBJECT for a SEARCH request, +- * it will not have sent back a result - otherwise, it will have +- * sent a result */ ++ * it will not have sent back a result - otherwise, it will have ++ * sent a result */ + rc = SLAPI_FAIL_GENERAL; + slapi_pblock_get(pb, SLAPI_RESULT_CODE, &err); + if (err == LDAP_NO_SUCH_OBJECT) { + /* may be the object exist somewhere else +- * wait the end of the loop to send back this error +- */ +- flag_no_such_object = 1; ++ * wait the end of the loop to send back this error ++ */ ++ flag_no_such_object = true; + } else { + /* err something other than LDAP_NO_SUCH_OBJECT, so the backend will +- * have sent the result - +- * Set a flag here so we don't return another result. */ ++ * have sent the result - ++ * Set a flag here so we don't return another result. */ + sent_result = 1; + } +- /* fall through */ ++ /* fall through */ + + case -1: /* an error occurred */ ++ slapi_pblock_get(pb, SLAPI_RESULT_CODE, &err); + /* PAGED RESULTS */ + if (op_is_pagedresults(operation)) { + /* cleanup the slot */ + pthread_mutex_lock(pagedresults_mutex); ++ if (err != LDAP_NO_SUCH_OBJECT && !flag_no_such_object) { ++ /* Free the results if not "no_such_object" */ ++ void *sr = NULL; ++ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); ++ be->be_search_results_release(&sr); ++ } + pagedresults_set_search_result(pb_conn, operation, NULL, 1, pr_idx); + rc = pagedresults_set_current_be(pb_conn, NULL, pr_idx, 1); + pthread_mutex_unlock(pagedresults_mutex); + } +- if (1 == flag_no_such_object) { +- break; +- } +- slapi_pblock_get(pb, SLAPI_RESULT_CODE, &err); +- if (err == LDAP_NO_SUCH_OBJECT) { +- /* may be the object exist somewhere else +- * wait the end of the loop to send back this error +- */ +- flag_no_such_object = 1; ++ ++ if (err == LDAP_NO_SUCH_OBJECT || flag_no_such_object) { ++ /* Maybe the object exists somewhere else, wait to the end ++ * of the loop to send back this error */ ++ flag_no_such_object = true; + break; + } else { +- /* for error other than LDAP_NO_SUCH_OBJECT +- * the error has already been sent +- * stop the search here +- */ ++ /* For error other than LDAP_NO_SUCH_OBJECT the error has ++ * already been sent stop the search here */ + cache_return_target_entry(pb, be, operation); + goto free_and_return; + } + + /* when rc == SLAPI_FAIL_DISKFULL this case is executed */ +- + case SLAPI_FAIL_DISKFULL: + operation_out_of_disk_space(); + cache_return_target_entry(pb, be, operation); +diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c +index db87e486e..4aa1fa3e5 100644 +--- a/ldap/servers/slapd/pagedresults.c ++++ b/ldap/servers/slapd/pagedresults.c +@@ -121,12 +121,15 @@ pagedresults_parse_control_value(Slapi_PBlock *pb, + if (ber_scanf(ber, "{io}", pagesize, &cookie) == LBER_ERROR) { + slapi_log_err(SLAPI_LOG_ERR, "pagedresults_parse_control_value", + "<= corrupted control value\n"); ++ ber_free(ber, 1); + return LDAP_PROTOCOL_ERROR; + } + if (!maxreqs) { + slapi_log_err(SLAPI_LOG_ERR, "pagedresults_parse_control_value", + "Simple paged results requests per conn exceeded the limit: %d\n", + maxreqs); ++ ber_free(ber, 1); ++ slapi_ch_free_string(&cookie.bv_val); + return LDAP_UNWILLING_TO_PERFORM; + } + +@@ -376,6 +379,10 @@ pagedresults_free_one_msgid(Connection *conn, ber_int_t msgid, pthread_mutex_t * + } + prp->pr_flags |= CONN_FLAG_PAGEDRESULTS_ABANDONED; + prp->pr_flags &= ~CONN_FLAG_PAGEDRESULTS_PROCESSING; ++ if (conn->c_pagedresults.prl_count > 0) { ++ _pr_cleanup_one_slot(prp); ++ conn->c_pagedresults.prl_count--; ++ } + rc = 0; + break; + } +@@ -940,7 +947,9 @@ pagedresults_is_timedout_nolock(Connection *conn) + return 1; + } + } ++ + slapi_log_err(SLAPI_LOG_TRACE, "<-- pagedresults_is_timedout", "<= false 2\n"); ++ + return 0; + } + +diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h +index 072f6f962..469874fd1 100644 +--- a/ldap/servers/slapd/slap.h ++++ b/ldap/servers/slapd/slap.h +@@ -74,7 +74,7 @@ static char ptokPBE[34] = "Internal (Software) Token "; + #include + #include + #include +- ++#include + #include /* For timespec definitions */ + + /* Provides our int types and platform specific requirements. */ +-- +2.48.0 + diff --git a/SOURCES/0022-Issue-6509-Fix-cherry-pick-issue-race-condition-in-P.patch b/SOURCES/0022-Issue-6509-Fix-cherry-pick-issue-race-condition-in-P.patch new file mode 100644 index 0000000..37ff6c4 --- /dev/null +++ b/SOURCES/0022-Issue-6509-Fix-cherry-pick-issue-race-condition-in-P.patch @@ -0,0 +1,29 @@ +From 27cd055197bc3cae458a1f86621aa5410c66dd2c Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Mon, 20 Jan 2025 15:51:24 -0500 +Subject: [PATCH] Issue 6509 - Fix cherry pick issue (race condition in Paged + results) + +Relates: https://github.com/389ds/389-ds-base/issues/6509 +--- + ldap/servers/slapd/daemon.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c +index 13dfe250d..57e07e5f5 100644 +--- a/ldap/servers/slapd/daemon.c ++++ b/ldap/servers/slapd/daemon.c +@@ -1620,8 +1620,8 @@ setup_pr_read_pds(Connection_Table *ct) + connection_table_move_connection_out_of_active_list(ct, c); + } else if (c->c_prfd != NULL) { + if ((!c->c_gettingber) && (c->c_threadnumber < c->c_max_threads_per_conn)) { +- ct->fd[listnum][count].fd = c->c_prfd; +- ct->fd[listnum][count].in_flags = SLAPD_POLL_FLAGS; ++ ct->fd[count].fd = c->c_prfd; ++ ct->fd[count].in_flags = SLAPD_POLL_FLAGS; + /* slot i of the connection table is mapped to slot + * count of the fds array */ + c->c_fdi = count; +-- +2.48.0 + diff --git a/SPECS/389-ds-base.spec b/SPECS/389-ds-base.spec index 0e9fecc..9a75c29 100644 --- a/SPECS/389-ds-base.spec +++ b/SPECS/389-ds-base.spec @@ -48,7 +48,7 @@ ExcludeArch: i686 Summary: 389 Directory Server (base) Name: 389-ds-base Version: 1.4.3.39 -Release: %{?relprefix}10%{?prerel}%{?dist} +Release: %{?relprefix}11%{?prerel}%{?dist} License: GPLv3+ and (ASL 2.0 or MIT) URL: https://www.port389.org Group: System Environment/Daemons @@ -313,6 +313,9 @@ Patch17: 0017-Issue-6224-Remove-test_referral_subsuffix-from-ds_lo.patc Patch18: 0018-Issue-6417-2nd-If-an-entry-RDN-is-identical-to-the-s.patch Patch19: 0019-Issue-6417-2nd-fix-typo.patch Patch20: 0020-Issue-6417-3rd-If-an-entry-RDN-is-identical-to-the-s.patch +Patch21: 0021-Issue-6509-Race-condition-with-Paged-Result-searches.patch +Patch22: 0022-Issue-6509-Fix-cherry-pick-issue-race-condition-in-P.patch + %description 389 Directory Server is an LDAPv3 compliant server. The base package includes the LDAP server and command line utilities for server administration. @@ -933,6 +936,9 @@ exit 0 %doc README.md %changelog +* Thu Jan 23 2025 Viktor Ashirov - 1.4.3.39-11 +- Resolves: RHEL-72487 - IPA LDAP error code T3 when no exceeded time limit from a paged search result [rhel-8.10.z] + * Fri Jan 17 2025 Viktor Ashirov - 1.4.3.39-10 - Resolves: RHEL-69822 - "Duplicated DN detected" errors when creating indexes or importing entries. [rhel-8.10.z] - Resolves: RHEL-71215 - Sub suffix causes "id2entry - Could not open id2entry err 0" error when the Directory Server starts [rhel-8.10.z]