From 3e192561860088e038bbfd40c13c46d0184ad8af Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Feb 20 2018 14:04:11 +0000 Subject: Ticket 49296 - Fix race condition in connection code with anonymous limits Bug Description: When a connection first comes in we set the anonymous resource limits (if set) before we do anything else. The way we check if the connection is "new" was flawed. It assumed the connection was new if no operations were completed yet, but there was a small window between sending the result and setting that the operation completed in the connection struct. So on a connection that binds and then does a search, when the server sends the bind result the client sends the search, but the search op/activity can be picked up before we set c_opscompleted. This opens a window where the code thinks the search op is the first op(new connection), and it incorrectly sets the anonymous limits for the bind dn. Fix description: Do not use c_opscompleted to determine if a connection is new, instead use a new flag to set the connection "initialized", which prevents the race condition. https://pagure.io/389-ds-base/issue/49296 Reviewed by: firstyear(Thanks!) --- diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c index 7e353e6..c1b4478 100644 --- a/ldap/servers/slapd/connection.c +++ b/ldap/servers/slapd/connection.c @@ -1557,7 +1557,8 @@ connection_threadmain() in connection_activity when the conn is added to the work queue, setup_pr_read_pds won't add the connection prfd to the poll list */ - if(pb_conn->c_opscompleted == 0){ + PR_EnterMonitor(pb_conn->c_mutex); + if(pb_conn->c_anonlimits_set == 0){ /* * We have a new connection, set the anonymous reslimit idletimeout * if applicable. @@ -1578,7 +1579,13 @@ connection_threadmain() } } slapi_ch_free_string( &anon_dn ); + /* + * Set connection as initialized to avoid setting anonymous limits + * multiple times on the same connection + */ + pb_conn->c_anonlimits_set = 1; } + PR_ExitMonitor(pb_conn->c_mutex); if (connection_call_io_layer_callbacks(pb_conn)) { slapi_log_err(SLAPI_LOG_ERR, "connection_threadmain", "Could not add/remove IO layers from connection\n"); diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 5469b51..7bf3d80 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1592,6 +1592,7 @@ typedef struct conn { PRUint64 c_maxthreadsblocked; /* # of operations blocked by maxthreads */ int c_opsinitiated; /* # ops initiated/next op id */ PRInt32 c_opscompleted; /* # ops completed */ + uint64_t c_anonlimits_set; /* default anon limits are set */ PRInt32 c_threadnumber; /* # threads used in this conn */ int c_refcnt; /* # ops refering to this conn */ PRMonitor *c_mutex; /* protect each conn structure; need to be re-entrant */