rpms/389-ds-base
5
0
Fork 0
Code Issues Pull Requests Releases Activity

Import from CS git

Browse Source
This commit is contained in:
eabdullin 2025-02-04 07:17:12 +00:00
parent 27a2aba684
commit fdd80910b8
3 changed files with 333 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

297
SOURCES/0021-Issue-6509-Race-condition-with-Paged-Result-searches.patch Normal file
View File

@ -0,0 +1,297 @@
From d2f9dd82e3610ee9b73feea981c680c03bb21394 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
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 <sys/stat.h>
#include <sys/socket.h>
#include <netinet/in.h>
-
+#include <stdbool.h>
#include <time.h> /* For timespec definitions */
/* Provides our int types and platform specific requirements. */
--
2.48.0

29
SOURCES/0022-Issue-6509-Fix-cherry-pick-issue-race-condition-in-P.patch Normal file
View File

@ -0,0 +1,29 @@
From 27cd055197bc3cae458a1f86621aa5410c66dd2c Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
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

8
SPECS/389-ds-base.spec
View File

@ -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 <vashirov@redhat.com> - 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 <vashirov@redhat.com> - 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]