|
 |
96373c |
From dd12327d1523f3ff9d6ae8b44b640fb9d0d2d53b Mon Sep 17 00:00:00 2001
|
|
|
96373c |
From: Mark Reynolds <mreynolds@redhat.com>
|
|
|
96373c |
Date: Mon, 19 Feb 2018 10:44:36 -0500
|
|
|
96373c |
Subject: [PATCH] Ticket 49296 - Fix race condition in connection code with
|
|
|
96373c |
anonymous limits
|
|
|
96373c |
|
|
|
96373c |
Bug Description: When a connection first comes in we set the anonymous
|
|
|
96373c |
resource limits (if set) before we do anything else. The
|
|
|
96373c |
way we check if the connection is "new" was flawed. It
|
|
|
96373c |
assumed the connection was new if no operations were
|
|
|
96373c |
completed yet, but there was a small window between sending
|
|
|
96373c |
the result and setting that the operation completed in the
|
|
|
96373c |
connection struct.
|
|
|
96373c |
|
|
|
96373c |
So on a connection that binds and then does a search, when
|
|
|
96373c |
the server sends the bind result the client sends the search,
|
|
|
96373c |
but the search op/activity can be picked up before we set
|
|
|
96373c |
c_opscompleted. This opens a window where the code thinks
|
|
|
96373c |
the search op is the first op(new connection), and it incorrectly
|
|
|
96373c |
sets the anonymous limits for the bind dn.
|
|
|
96373c |
|
|
|
96373c |
Fix description: Do not use c_opscompleted to determine if a connection is new,
|
|
|
96373c |
instead use a new flag to set the connection "initialized",
|
|
|
96373c |
which prevents the race condition.
|
|
|
96373c |
|
|
|
96373c |
https://pagure.io/389-ds-base/issue/49296
|
|
|
96373c |
|
|
|
96373c |
Reviewed by: firstyear(Thanks!)
|
|
|
96373c |
|
|
|
96373c |
(cherry picked from commit 0d5214d08e6b5b39fb9d5ef5cf3d8834574954f1)
|
|
|
96373c |
---
|
|
|
96373c |
ldap/servers/slapd/connection.c | 12 +++++++++++-
|
|
|
96373c |
ldap/servers/slapd/slap.h | 7 +++----
|
|
|
96373c |
2 files changed, 14 insertions(+), 5 deletions(-)
|
|
|
96373c |
|
|
|
96373c |
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
|
|
|
96373c |
index 5d2b64ed2..5ca32a333 100644
|
|
|
96373c |
--- a/ldap/servers/slapd/connection.c
|
|
|
96373c |
+++ b/ldap/servers/slapd/connection.c
|
|
|
96373c |
@@ -217,6 +217,7 @@ connection_cleanup(Connection *conn)
|
|
|
96373c |
conn->c_connid = 0;
|
|
|
96373c |
conn->c_opsinitiated = 0;
|
|
|
96373c |
conn->c_opscompleted = 0;
|
|
|
96373c |
+ conn->c_anonlimits_set = 0;
|
|
|
96373c |
conn->c_threadnumber = 0;
|
|
|
96373c |
conn->c_refcnt = 0;
|
|
|
96373c |
conn->c_idlesince = 0;
|
|
|
96373c |
@@ -1549,7 +1550,9 @@ connection_threadmain()
|
|
|
96373c |
g_decr_active_threadcnt();
|
|
|
96373c |
return;
|
|
|
96373c |
}
|
|
|
96373c |
- if (pb_conn->c_opscompleted == 0) {
|
|
|
96373c |
+
|
|
|
96373c |
+ PR_EnterMonitor(pb_conn->c_mutex);
|
|
|
96373c |
+ if (pb_conn->c_anonlimits_set == 0) {
|
|
|
96373c |
/*
|
|
|
96373c |
* We have a new connection, set the anonymous reslimit idletimeout
|
|
|
96373c |
* if applicable.
|
|
|
96373c |
@@ -1568,7 +1571,14 @@ connection_threadmain()
|
|
|
96373c |
}
|
|
|
96373c |
}
|
|
|
96373c |
slapi_ch_free_string(&anon_dn);
|
|
|
96373c |
+ /*
|
|
|
96373c |
+ * Set connection as initialized to avoid setting anonymous limits
|
|
|
96373c |
+ * multiple times on the same connection
|
|
|
96373c |
+ */
|
|
|
96373c |
+ pb_conn->c_anonlimits_set = 1;
|
|
|
96373c |
}
|
|
|
96373c |
+ PR_ExitMonitor(pb_conn->c_mutex);
|
|
|
96373c |
+
|
|
|
96373c |
if (connection_call_io_layer_callbacks(pb_conn)) {
|
|
|
96373c |
slapi_log_err(SLAPI_LOG_ERR, "connection_threadmain",
|
|
|
96373c |
"Could not add/remove IO layers from connection\n");
|
|
|
96373c |
diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h
|
|
|
96373c |
index 9b10aa19e..03355f5fe 100644
|
|
|
96373c |
--- a/ldap/servers/slapd/slap.h
|
|
|
96373c |
+++ b/ldap/servers/slapd/slap.h
|
|
|
96373c |
@@ -1616,6 +1616,7 @@ typedef struct conn
|
|
|
96373c |
PRUint64 c_maxthreadsblocked; /* # of operations blocked by maxthreads */
|
|
|
96373c |
int c_opsinitiated; /* # ops initiated/next op id */
|
|
|
96373c |
PRInt32 c_opscompleted; /* # ops completed */
|
|
|
96373c |
+ uint64_t c_anonlimits_set; /* default anon limits are set */
|
|
|
96373c |
PRInt32 c_threadnumber; /* # threads used in this conn */
|
|
|
96373c |
int c_refcnt; /* # ops refering to this conn */
|
|
|
96373c |
PRMonitor *c_mutex; /* protect each conn structure; need to be re-entrant */
|
|
|
96373c |
@@ -1623,10 +1624,8 @@ typedef struct conn
|
|
|
96373c |
time_t c_idlesince; /* last time of activity on conn */
|
|
|
96373c |
int c_idletimeout; /* local copy of idletimeout */
|
|
|
96373c |
int c_idletimeout_handle; /* the resource limits handle */
|
|
|
96373c |
- Conn_private *c_private; /* data which is not shared outside*/
|
|
|
96373c |
- /* connection.c */
|
|
|
96373c |
- int c_flags; /* Misc flags used only for SSL */
|
|
|
96373c |
- /* status currently */
|
|
|
96373c |
+ Conn_private *c_private; /* data which is not shared outside connection.c */
|
|
|
96373c |
+ int c_flags; /* Misc flags used only for SSL status currently */
|
|
|
96373c |
int c_needpw; /* need new password */
|
|
|
96373c |
CERTCertificate *c_client_cert; /* Client's Cert */
|
|
|
96373c |
PRFileDesc *c_prfd; /* NSPR 2.1 FileDesc */
|
|
|
96373c |
--
|
|
|
96373c |
2.13.6
|
|
|
96373c |
|