[389-ds-base] branch 389-ds-base-1.4.3 updated: Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
new 632ed56 Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
632ed56 is described below
commit 632ed5692dec13dfc8a8bebe55b649b9100cf36a
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Fri May 8 15:05:25 2020 -0400
Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
Description:
FreeIPA LDAP update code relies on the schema retrieval when
deciding what to do with values of single-valued LDAP attributes.
In the case attribute is single-valued and some value was present
in the original entry for this attribute, it would use MOD_REPLACE.
Otherwise, it uses MOD_DELETE + MOD_ADD.
Many attributes used in cn=config entries have no formal schema
defined. Since by default an attribute is multi-valued, this fails
the logic above for actual single-valued attributes, like
nsslapd-enable-upgrade-hash. It means FreeIPA has to write special
logic to handle just this attribute.
It would be good to expose schema for nsslapd-enable-upgrade-hash.
We need to change its value to off in all FreeIPA installations
because ipa-pwd-extop plugin prevents hashed passwords in updates
due to a need to regenerate Kerberos hashes on a password change.
It means upgrade of a password hash on LDAP bind will never work
in FreeIPA.
Note - this does move us closer to our goal of adding all the
configuration attributes to the schema.
fixes: https://pagure.io/389-ds-base/issue/51078
Reviewed by: mreynolds (one line commit rule)
---
ldap/schema/01core389.ldif | 1 +
1 file changed, 1 insertion(+)
diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif
index c1fd7f5..b5478cb 100644
--- a/ldap/schema/01core389.ldif
+++ b/ldap/schema/01core389.ldif
@@ -317,6 +317,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2365 NAME 'nsds5replicaLastUpdateStatusJ
attributeTypes: ( 2.16.840.1.113730.3.1.2367 NAME 'nsslapd-libPath' DESC 'Rewriter shared library path' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2368 NAME 'nsslapd-filterrewriter' DESC 'Filter rewriter function name' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2369 NAME 'nsslapd-returnedAttrRewriter' DESC 'Returned attribute rewriter function name' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
+attributeTypes: ( 2.16.840.1.113730.3.1.2370 NAME 'nsslapd-enable-upgrade-hash' DESC 'Upgrade password hash on bind' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
#
# objectclasses
#
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch master updated: Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch master
in repository 389-ds-base.
The following commit(s) were added to refs/heads/master by this push:
new 6a0ece1 Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
6a0ece1 is described below
commit 6a0ece1e5d6c3a6f8d0d4f312a8e3af6f518087d
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Fri May 8 15:05:25 2020 -0400
Issue 51078 - Add nsslapd-enable-upgrade-hash to the schema
Description:
FreeIPA LDAP update code relies on the schema retrieval when
deciding what to do with values of single-valued LDAP attributes.
In the case attribute is single-valued and some value was present
in the original entry for this attribute, it would use MOD_REPLACE.
Otherwise, it uses MOD_DELETE + MOD_ADD.
Many attributes used in cn=config entries have no formal schema
defined. Since by default an attribute is multi-valued, this fails
the logic above for actual single-valued attributes, like
nsslapd-enable-upgrade-hash. It means FreeIPA has to write special
logic to handle just this attribute.
It would be good to expose schema for nsslapd-enable-upgrade-hash.
We need to change its value to off in all FreeIPA installations
because ipa-pwd-extop plugin prevents hashed passwords in updates
due to a need to regenerate Kerberos hashes on a password change.
It means upgrade of a password hash on LDAP bind will never work
in FreeIPA.
Note - this does move us closer to our goal of adding all the
configuration attributes to the schema.
fixes: https://pagure.io/389-ds-base/issue/51078
Reviewed by: mreynolds (one line commit rule)
---
ldap/schema/01core389.ldif | 1 +
1 file changed, 1 insertion(+)
diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif
index c1fd7f5..b5478cb 100644
--- a/ldap/schema/01core389.ldif
+++ b/ldap/schema/01core389.ldif
@@ -317,6 +317,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2365 NAME 'nsds5replicaLastUpdateStatusJ
attributeTypes: ( 2.16.840.1.113730.3.1.2367 NAME 'nsslapd-libPath' DESC 'Rewriter shared library path' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2368 NAME 'nsslapd-filterrewriter' DESC 'Filter rewriter function name' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2369 NAME 'nsslapd-returnedAttrRewriter' DESC 'Returned attribute rewriter function name' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
+attributeTypes: ( 2.16.840.1.113730.3.1.2370 NAME 'nsslapd-enable-upgrade-hash' DESC 'Upgrade password hash on bind' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
#
# objectclasses
#
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.3 updated: Issue 51054 - Revise ACI target syntax checking
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
new f2e9961 Issue 51054 - Revise ACI target syntax checking
f2e9961 is described below
commit f2e9961f1acc657b13bc901bf1c3f58f735d1400
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Thu Apr 30 11:47:26 2020 -0400
Issue 51054 - Revise ACI target syntax checking
Bug Description: The previous commit enforced a strict syntax that was previously
allowed. This is causing regressions for customers and community
members.
Fix Description: Reject ACI's that use more than one equal sign between the target
keyword and the value, but do not enforce that the values are
quoted. A flag was added that we can turn on strict syntax at a
later date, but for now we will continue allow values without quotes.
relates: https://pagure.io/389-ds-base/issue/51054
Reviewed by: firstyear & spichugi(Thanks!!)
---
dirsrvtests/tests/suites/acl/syntax_test.py | 2 +-
ldap/servers/plugins/acl/aclparse.c | 59 ++++++++++++++++++++++++-----
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/dirsrvtests/tests/suites/acl/syntax_test.py b/dirsrvtests/tests/suites/acl/syntax_test.py
index cf08c65..c143ff7 100644
--- a/dirsrvtests/tests/suites/acl/syntax_test.py
+++ b/dirsrvtests/tests/suites/acl/syntax_test.py
@@ -192,7 +192,7 @@ FAILED = [('test_targattrfilters_18',
f'(all)userdn="ldap:///anyone";)'), ]
-(a)pytest.mark.skipif(ds_is_older('1.4.3'), reason="Not implemented")
+(a)pytest.mark.xfail(reason='https://bugzilla.redhat.com/show_bug.cgi?id=1691473')
@pytest.mark.parametrize("real_value", [a[1] for a in FAILED],
ids=[a[0] for a in FAILED])
def test_aci_invalid_syntax_fail(topo, real_value):
diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
index d5edaf5..577251e 100644
--- a/ldap/servers/plugins/acl/aclparse.c
+++ b/ldap/servers/plugins/acl/aclparse.c
@@ -34,6 +34,10 @@ static int acl_verify_exactly_one_attribute(char *attr_name, Slapi_Filter *f);
static int type_compare(Slapi_Filter *f, void *arg);
static int acl_check_for_target_macro(aci_t *aci_item, char *value);
static int get_acl_rights_as_int(char *strValue);
+
+/* Enforce strict aci syntax */
+#define STRICT_SYNTAX_CHECK 0
+
/***************************************************************************
*
* acl_parse
@@ -306,11 +310,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
if (NULL == tmpstr) {
return ACL_SYNTAX_ERR;
}
+
tmpstr++;
+ /* Consecutive equals are not allowed */
+ if (*tmpstr == '=') {
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+ "__aclp__parse_aci - target filter has an invalid syntax, "
+ "do not use more than one '=' between the targetfilter keyword and its value: (%s)\n",
+ str);
+ return ACL_SYNTAX_ERR;
+ }
__acl_strip_leading_space(&tmpstr);
/* The first character is expected to be a double quote */
- if (*tmpstr != '"') {
+ if (STRICT_SYNTAX_CHECK && *tmpstr != '"') {
slapi_log_err(SLAPI_LOG_ERR, plugin_name,
"__aclp__parse_aci - target filter has an invalid value (%s)\n", str);
return ACL_SYNTAX_ERR;
@@ -355,11 +368,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
strncpy(s, single_space, 1);
}
if ((s = strchr(str, '=')) != NULL) {
- value = s + 1;
+ s++;
+ if (*s == '=') {
+ /* Consecutive equals are not allowed */
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+ "__aclp__parse_aci - target to/from has an invalid syntax, "
+ "do not use more than one '=' between the target to/from keyword and its value: (%s)\n",
+ str);
+ return ACL_SYNTAX_ERR;
+ }
+ value = s;
__acl_strip_leading_space(&value);
__acl_strip_trailing_space(value);
/* The first character is expected to be a double quote */
- if (*value != '"') {
+ if (STRICT_SYNTAX_CHECK && *value != '"') {
slapi_log_err(SLAPI_LOG_ERR, plugin_name,
"__aclp__parse_aci - target to/from has an invalid value (%s)\n", str);
return ACL_SYNTAX_ERR;
@@ -414,11 +436,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
strncpy(s, single_space, 1);
}
if ((s = strchr(str, '=')) != NULL) {
- value = s + 1;
+ s++;
+ if (*s == '=') {
+ /* Consecutive equals are not allowed */
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+ "__aclp__parse_aci - target has an invalid syntax, "
+ "do not use more than one '=' between the target keyword and its value: (%s)\n",
+ str);
+ return ACL_SYNTAX_ERR;
+ }
+ value = s;
__acl_strip_leading_space(&value);
__acl_strip_trailing_space(value);
/* The first character is expected to be a double quote */
- if (*value != '"') {
+ if (STRICT_SYNTAX_CHECK && *value != '"') {
slapi_log_err(SLAPI_LOG_ERR, plugin_name,
"__aclp__parse_aci - target has an invalid value (%s)\n", str);
return ACL_SYNTAX_ERR;
@@ -1520,14 +1551,22 @@ __aclp__init_targetattr(aci_t *aci, char *attr_val, char **errbuf)
return ACL_SYNTAX_ERR;
}
s++;
+ if (*s == '=') {
+ /* Consecutive equals are not allowed */
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+ "__aclp__init_targetattr - targetattr has an invalid syntax, "
+ "do not use more than one '=' between the targetattr and its value: (%s)\n",
+ attr_val);
+ return ACL_SYNTAX_ERR;
+ }
__acl_strip_leading_space(&s);
__acl_strip_trailing_space(s);
len = strlen(s);
- /* Simple targetattr statements may not be quoted e.g.
- targetattr=* or targetattr=userPassword
- if it begins with a quote, it must end with one as well
- */
+ /*
+ * If it begins with a quote, it must end with one as well
+ */
if (*s == '"') {
+
if (s[len - 1] == '"') {
s[len - 1] = '\0'; /* trim trailing quote */
} else {
@@ -1545,7 +1584,7 @@ __aclp__init_targetattr(aci_t *aci, char *attr_val, char **errbuf)
return ACL_SYNTAX_ERR;
}
s++; /* skip leading quote */
- } else {
+ } else if (STRICT_SYNTAX_CHECK) {
/* The first character is expected to be a double quote */
slapi_log_err(SLAPI_LOG_ERR, plugin_name,
"__aclp__init_targetattr - targetattr has an invalid value (%s)\n", attr_val);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.3.10 updated: Ticket 51068 - deadlock when updating the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.3.10
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.3.10 by this push:
new 98826d9 Ticket 51068 - deadlock when updating the schema
98826d9 is described below
commit 98826d9cf9e087f19186e002289cfd7dc9a68c12
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Tue May 5 17:44:01 2020 +0200
Ticket 51068 - deadlock when updating the schema
Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when it
calls schema_changed_callback. So the trigger is a change of schema (direct or via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock
Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock
In order to prevent writer starvation a new slapi_new_rwlock_prio
create rwlock with priority to writers.
https://pagure.io/389-ds-base/issue/51068
Reviewed by: Mark Reynolds, William Brown
Platforms tested: 30
Flag Day: no
Doc impact: no
---
ldap/servers/plugins/statechange/statechange.c | 24 +++++++++++++-----------
ldap/servers/slapd/slapi-plugin.h | 16 ++++++++++++++++
ldap/servers/slapd/slapi2nspr.c | 25 +++++++++++++++++++++++++
ldap/servers/slapd/vattr.c | 2 +-
4 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/ldap/servers/plugins/statechange/statechange.c b/ldap/servers/plugins/statechange/statechange.c
index f89b394..0a3838b 100644
--- a/ldap/servers/plugins/statechange/statechange.c
+++ b/ldap/servers/plugins/statechange/statechange.c
@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
#define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
static PRUint64 g_plugin_started = 0;
/*
@@ -140,7 +140,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
api[3] = (void *)_statechange_unregister_all;
api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
- if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+ if (0 == (buffer_lock = slapi_new_rwlock()))
{
/* badness */
slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start - Failed to create lock\n");
@@ -180,7 +180,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
slapi_counter_destroy(&op_counter);
slapi_apib_unregister(StateChange_v1_0_GUID);
- slapi_destroy_mutex(buffer_lock);
+ if (buffer_lock) {
+ slapi_destroy_rwlock(buffer_lock);
+ }
buffer_lock = NULL;
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_close\n");
@@ -240,7 +242,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "--> statechange_post_op\n");
/* evaluate this operation against the notification entries */
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_rdlock(buffer_lock);
if (head) {
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (NULL == sdn) {
@@ -290,7 +292,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
} while (notify && notify != head);
}
bail:
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_post_op\n");
return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -338,7 +340,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
}
item->func = func;
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (head == NULL) {
head = item;
head->next = head;
@@ -349,7 +351,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
head->prev = item;
item->prev->next = item;
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_ch_free_string(&writable_filter);
ret = SLAPI_PLUGIN_SUCCESS;
@@ -371,7 +373,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
return ret;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if ((func = statechange_find_notify(dn, filter, thefunc))) {
func->prev->next = func->next;
@@ -392,7 +394,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
slapi_ch_free((void **)&func);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
return ret;
@@ -410,7 +412,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
return;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (notify) {
do {
@@ -441,7 +443,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
} while (notify != start_notify && notify != NULL);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 679bdbb..865d83b 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6127,6 +6127,22 @@ int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval *timeout);
int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
+/**
* Creates a new read/write lock.
*
* \return A pointer to a \c Slapi_RWLock
diff --git a/ldap/servers/slapd/slapi2nspr.c b/ldap/servers/slapd/slapi2nspr.c
index b3e6d94..232d159 100644
--- a/ldap/servers/slapd/slapi2nspr.c
+++ b/ldap/servers/slapd/slapi2nspr.c
@@ -182,6 +182,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
}
Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+ pthread_rwlock_t *rwlock = NULL;
+ pthread_rwlockattr_t attr;
+
+ pthread_rwlockattr_init(&attr);
+ if (prio_writer) {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ } else {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+ }
+
+ rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+ if (rwlock) {
+ pthread_rwlock_init(rwlock, &attr);
+ }
+
+ return ((Slapi_RWLock *)rwlock);
+#else
+ return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE, "slapi_rwlock"));
+#endif
+}
+
+Slapi_RWLock *
slapi_new_rwlock(void)
{
#ifdef USE_POSIX_RWLOCKS
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index d8b2c83..7c70f41 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -1996,7 +1996,7 @@ vattr_map_create(void)
return ENOMEM;
}
- the_map->lock = slapi_new_rwlock();
+ the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
if (NULL == the_map) {
slapd_nasty(sourcefile, 3, 0);
return ENOMEM;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.1 updated: Ticket 51068 - deadlock when updating the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
new d3c792b Ticket 51068 - deadlock when updating the schema
d3c792b is described below
commit d3c792baf0e8a05160c24d067c5832f6f7c65a20
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Tue May 5 17:44:01 2020 +0200
Ticket 51068 - deadlock when updating the schema
Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when it
calls schema_changed_callback. So the trigger is a change of schema (direct or via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock
Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock
In order to prevent writer starvation a new slapi_new_rwlock_prio
create rwlock with priority to writers.
https://pagure.io/389-ds-base/issue/51068
Reviewed by: Mark Reynolds, William Brown
Platforms tested: 30
Flag Day: no
Doc impact: no
---
ldap/servers/plugins/statechange/statechange.c | 24 +++++++++++++-----------
ldap/servers/slapd/slapi-plugin.h | 16 ++++++++++++++++
ldap/servers/slapd/slapi2nspr.c | 25 +++++++++++++++++++++++++
ldap/servers/slapd/vattr.c | 2 +-
4 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/ldap/servers/plugins/statechange/statechange.c b/ldap/servers/plugins/statechange/statechange.c
index c6a611d..9026382 100644
--- a/ldap/servers/plugins/statechange/statechange.c
+++ b/ldap/servers/plugins/statechange/statechange.c
@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
#define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
static PRUint64 g_plugin_started = 0;
/*
@@ -139,7 +139,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
api[3] = (void *)_statechange_unregister_all;
api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
- if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+ if (0 == (buffer_lock = slapi_new_rwlock()))
{
/* badness */
slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start - Failed to create lock\n");
@@ -179,7 +179,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
slapi_counter_destroy(&op_counter);
slapi_apib_unregister(StateChange_v1_0_GUID);
- slapi_destroy_mutex(buffer_lock);
+ if (buffer_lock) {
+ slapi_destroy_rwlock(buffer_lock);
+ }
buffer_lock = NULL;
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_close\n");
@@ -239,7 +241,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "--> statechange_post_op\n");
/* evaluate this operation against the notification entries */
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_rdlock(buffer_lock);
if (head) {
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (NULL == sdn) {
@@ -289,7 +291,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
} while (notify && notify != head);
}
bail:
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_post_op\n");
return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -337,7 +339,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
}
item->func = func;
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (head == NULL) {
head = item;
head->next = head;
@@ -348,7 +350,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
head->prev = item;
item->prev->next = item;
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_ch_free_string(&writable_filter);
ret = SLAPI_PLUGIN_SUCCESS;
@@ -370,7 +372,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
return ret;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if ((func = statechange_find_notify(dn, filter, thefunc))) {
func->prev->next = func->next;
@@ -391,7 +393,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
slapi_ch_free((void **)&func);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
return ret;
@@ -409,7 +411,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
return;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (notify) {
do {
@@ -440,7 +442,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
} while (notify != start_notify && notify != NULL);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 596824c..ff32f87 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6087,6 +6087,22 @@ int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval *timeout);
int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
+/**
* Creates a new read/write lock.
*
* \return A pointer to a \c Slapi_RWLock
diff --git a/ldap/servers/slapd/slapi2nspr.c b/ldap/servers/slapd/slapi2nspr.c
index b3e6d94..232d159 100644
--- a/ldap/servers/slapd/slapi2nspr.c
+++ b/ldap/servers/slapd/slapi2nspr.c
@@ -182,6 +182,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
}
Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+ pthread_rwlock_t *rwlock = NULL;
+ pthread_rwlockattr_t attr;
+
+ pthread_rwlockattr_init(&attr);
+ if (prio_writer) {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ } else {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+ }
+
+ rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+ if (rwlock) {
+ pthread_rwlock_init(rwlock, &attr);
+ }
+
+ return ((Slapi_RWLock *)rwlock);
+#else
+ return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE, "slapi_rwlock"));
+#endif
+}
+
+Slapi_RWLock *
slapi_new_rwlock(void)
{
#ifdef USE_POSIX_RWLOCKS
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index d8b2c83..7c70f41 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -1996,7 +1996,7 @@ vattr_map_create(void)
return ENOMEM;
}
- the_map->lock = slapi_new_rwlock();
+ the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
if (NULL == the_map) {
slapd_nasty(sourcefile, 3, 0);
return ENOMEM;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.2 updated: Ticket 51068 - deadlock when updating the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.4.2
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.2 by this push:
new b4d2e3a Ticket 51068 - deadlock when updating the schema
b4d2e3a is described below
commit b4d2e3a302abe53cb8308c4119ea21f5d5cc525e
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Tue May 5 17:44:01 2020 +0200
Ticket 51068 - deadlock when updating the schema
Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when it
calls schema_changed_callback. So the trigger is a change of schema (direct or via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock
Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock
In order to prevent writer starvation a new slapi_new_rwlock_prio
create rwlock with priority to writers.
https://pagure.io/389-ds-base/issue/51068
Reviewed by: Mark Reynolds, William Brown
Platforms tested: 30
Flag Day: no
Doc impact: no
---
ldap/servers/plugins/statechange/statechange.c | 24 +++++++++++++-----------
ldap/servers/slapd/slapi-plugin.h | 16 ++++++++++++++++
ldap/servers/slapd/slapi2nspr.c | 25 +++++++++++++++++++++++++
ldap/servers/slapd/vattr.c | 2 +-
4 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/ldap/servers/plugins/statechange/statechange.c b/ldap/servers/plugins/statechange/statechange.c
index c6a611d..9026382 100644
--- a/ldap/servers/plugins/statechange/statechange.c
+++ b/ldap/servers/plugins/statechange/statechange.c
@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
#define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
static PRUint64 g_plugin_started = 0;
/*
@@ -139,7 +139,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
api[3] = (void *)_statechange_unregister_all;
api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
- if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+ if (0 == (buffer_lock = slapi_new_rwlock()))
{
/* badness */
slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start - Failed to create lock\n");
@@ -179,7 +179,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
slapi_counter_destroy(&op_counter);
slapi_apib_unregister(StateChange_v1_0_GUID);
- slapi_destroy_mutex(buffer_lock);
+ if (buffer_lock) {
+ slapi_destroy_rwlock(buffer_lock);
+ }
buffer_lock = NULL;
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_close\n");
@@ -239,7 +241,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "--> statechange_post_op\n");
/* evaluate this operation against the notification entries */
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_rdlock(buffer_lock);
if (head) {
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (NULL == sdn) {
@@ -289,7 +291,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
} while (notify && notify != head);
}
bail:
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_post_op\n");
return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -337,7 +339,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
}
item->func = func;
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (head == NULL) {
head = item;
head->next = head;
@@ -348,7 +350,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
head->prev = item;
item->prev->next = item;
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_ch_free_string(&writable_filter);
ret = SLAPI_PLUGIN_SUCCESS;
@@ -370,7 +372,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
return ret;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if ((func = statechange_find_notify(dn, filter, thefunc))) {
func->prev->next = func->next;
@@ -391,7 +393,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
slapi_ch_free((void **)&func);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
return ret;
@@ -409,7 +411,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
return;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (notify) {
do {
@@ -440,7 +442,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
} while (notify != start_notify && notify != NULL);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 396d084..623dad1 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6087,6 +6087,22 @@ int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval *timeout);
int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
+/**
* Creates a new read/write lock.
*
* \return A pointer to a \c Slapi_RWLock
diff --git a/ldap/servers/slapd/slapi2nspr.c b/ldap/servers/slapd/slapi2nspr.c
index b3e6d94..232d159 100644
--- a/ldap/servers/slapd/slapi2nspr.c
+++ b/ldap/servers/slapd/slapi2nspr.c
@@ -182,6 +182,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
}
Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+ pthread_rwlock_t *rwlock = NULL;
+ pthread_rwlockattr_t attr;
+
+ pthread_rwlockattr_init(&attr);
+ if (prio_writer) {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ } else {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+ }
+
+ rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+ if (rwlock) {
+ pthread_rwlock_init(rwlock, &attr);
+ }
+
+ return ((Slapi_RWLock *)rwlock);
+#else
+ return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE, "slapi_rwlock"));
+#endif
+}
+
+Slapi_RWLock *
slapi_new_rwlock(void)
{
#ifdef USE_POSIX_RWLOCKS
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index d8b2c83..7c70f41 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -1996,7 +1996,7 @@ vattr_map_create(void)
return ENOMEM;
}
- the_map->lock = slapi_new_rwlock();
+ the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
if (NULL == the_map) {
slapd_nasty(sourcefile, 3, 0);
return ENOMEM;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.3 updated: Ticket 51068 - deadlock when updating the schema
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
new d73cdd1 Ticket 51068 - deadlock when updating the schema
d73cdd1 is described below
commit d73cdd12a95ab8b4f14513fbfc1a430260ab3c89
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Tue May 5 17:44:01 2020 +0200
Ticket 51068 - deadlock when updating the schema
Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when it
calls schema_changed_callback. So the trigger is a change of schema (direct or via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock
Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock
In order to prevent writer starvation a new slapi_new_rwlock_prio
create rwlock with priority to writers.
https://pagure.io/389-ds-base/issue/51068
Reviewed by: Mark Reynolds, William Brown
Platforms tested: 30
Flag Day: no
Doc impact: no
---
ldap/servers/plugins/statechange/statechange.c | 24 +++++++++++++-----------
ldap/servers/slapd/slapi-plugin.h | 16 ++++++++++++++++
ldap/servers/slapd/slapi2nspr.c | 25 +++++++++++++++++++++++++
ldap/servers/slapd/vattr.c | 2 +-
4 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/ldap/servers/plugins/statechange/statechange.c b/ldap/servers/plugins/statechange/statechange.c
index c6a611d..9026382 100644
--- a/ldap/servers/plugins/statechange/statechange.c
+++ b/ldap/servers/plugins/statechange/statechange.c
@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
#define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
static PRUint64 g_plugin_started = 0;
/*
@@ -139,7 +139,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
api[3] = (void *)_statechange_unregister_all;
api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
- if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+ if (0 == (buffer_lock = slapi_new_rwlock()))
{
/* badness */
slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start - Failed to create lock\n");
@@ -179,7 +179,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
slapi_counter_destroy(&op_counter);
slapi_apib_unregister(StateChange_v1_0_GUID);
- slapi_destroy_mutex(buffer_lock);
+ if (buffer_lock) {
+ slapi_destroy_rwlock(buffer_lock);
+ }
buffer_lock = NULL;
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_close\n");
@@ -239,7 +241,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "--> statechange_post_op\n");
/* evaluate this operation against the notification entries */
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_rdlock(buffer_lock);
if (head) {
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (NULL == sdn) {
@@ -289,7 +291,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
} while (notify && notify != head);
}
bail:
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_post_op\n");
return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -337,7 +339,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
}
item->func = func;
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (head == NULL) {
head = item;
head->next = head;
@@ -348,7 +350,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
head->prev = item;
item->prev->next = item;
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_ch_free_string(&writable_filter);
ret = SLAPI_PLUGIN_SUCCESS;
@@ -370,7 +372,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
return ret;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if ((func = statechange_find_notify(dn, filter, thefunc))) {
func->prev->next = func->next;
@@ -391,7 +393,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
slapi_ch_free((void **)&func);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
return ret;
@@ -409,7 +411,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
return;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (notify) {
do {
@@ -440,7 +442,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
} while (notify != start_notify && notify != NULL);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index e7ec581..0e38570 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6087,6 +6087,22 @@ int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval *timeout);
int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
+/**
* Creates a new read/write lock.
*
* \return A pointer to a \c Slapi_RWLock
diff --git a/ldap/servers/slapd/slapi2nspr.c b/ldap/servers/slapd/slapi2nspr.c
index b3e6d94..232d159 100644
--- a/ldap/servers/slapd/slapi2nspr.c
+++ b/ldap/servers/slapd/slapi2nspr.c
@@ -182,6 +182,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
}
Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+ pthread_rwlock_t *rwlock = NULL;
+ pthread_rwlockattr_t attr;
+
+ pthread_rwlockattr_init(&attr);
+ if (prio_writer) {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ } else {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+ }
+
+ rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+ if (rwlock) {
+ pthread_rwlock_init(rwlock, &attr);
+ }
+
+ return ((Slapi_RWLock *)rwlock);
+#else
+ return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE, "slapi_rwlock"));
+#endif
+}
+
+Slapi_RWLock *
slapi_new_rwlock(void)
{
#ifdef USE_POSIX_RWLOCKS
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index d8b2c83..7c70f41 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -1996,7 +1996,7 @@ vattr_map_create(void)
return ENOMEM;
}
- the_map->lock = slapi_new_rwlock();
+ the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
if (NULL == the_map) {
slapd_nasty(sourcefile, 3, 0);
return ENOMEM;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 10 months
[389-ds-base] branch 389-ds-base-1.4.1 updated: Issue 51060 - unable to set sslVersionMin to TLS1.0
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
new 8409e48 Issue 51060 - unable to set sslVersionMin to TLS1.0
8409e48 is described below
commit 8409e483353733e6b164f6c26e1102fdb2bd58ec
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Wed Apr 29 16:07:38 2020 -0400
Issue 51060 - unable to set sslVersionMin to TLS1.0
Description: When processing the "sslVersionMin" attribute we were incorrectly
setting it to TLS1.2 (current default level)
fixes: https://pagure.io/389-ds-base/issue/51060
Reviewed by: firstyear(Thanks!)
---
dirsrvtests/tests/suites/tls/ssl_version_test.py | 12 ++++++++++++
ldap/servers/slapd/ssl.c | 4 ++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/dirsrvtests/tests/suites/tls/ssl_version_test.py b/dirsrvtests/tests/suites/tls/ssl_version_test.py
index acc8b23..d9dae5a 100644
--- a/dirsrvtests/tests/suites/tls/ssl_version_test.py
+++ b/dirsrvtests/tests/suites/tls/ssl_version_test.py
@@ -19,10 +19,12 @@ def test_ssl_version_range(topo):
1. Get current default range
2. Set sslVersionMin and verify it is applied after a restart
3. Set sslVersionMax and verify it is applied after a restart
+ 4. Sanity test all the min/max versions
:expectedresults:
1. Success
2. Success
3. Success
+ 4. Success
"""
topo.standalone.enable_tls()
@@ -47,6 +49,16 @@ def test_ssl_version_range(topo):
max = enc.get_attr_val_utf8('sslVersionMax')
assert max == default_min
+ # Sanity test all the min/max versions
+ for attr, versions in [('sslVersionMin', ['TLS1.0', 'TLS1.1', 'TLS1.2', 'TLS1.0']),
+ ('sslVersionMax', ['TLS1.0', 'TLS1.1', 'TLS1.2'])]:
+ for version in versions:
+ # Test that the setting is correctly applied after a restart
+ enc.replace(attr, version)
+ topo.standalone.restart()
+ current_val = enc.get_attr_val_utf8(attr)
+ assert current_val == version
+
if __name__ == '__main__':
# Run isolated
diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c
index 9296cd4..0248585 100644
--- a/ldap/servers/slapd/ssl.c
+++ b/ldap/servers/slapd/ssl.c
@@ -1291,7 +1291,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emin);
(*rval) = enabledNSSVersions.min;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
} else {
if (enabledNSSVersions.max < CURRENT_DEFAULT_SSL_VERSION) {
@@ -1302,7 +1302,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emax);
(*rval) = enabledNSSVersions.max;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
}
} else if (tlsv < 1.2f) { /* TLS1.1 */
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 11 months
[389-ds-base] branch 389-ds-base-1.4.2 updated: Issue 51060 - unable to set sslVersionMin to TLS1.0
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.2
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.2 by this push:
new 01b7ef7 Issue 51060 - unable to set sslVersionMin to TLS1.0
01b7ef7 is described below
commit 01b7ef727e68acc2ed516d8dace59728b01d3e06
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Wed Apr 29 16:07:38 2020 -0400
Issue 51060 - unable to set sslVersionMin to TLS1.0
Description: When processing the "sslVersionMin" attribute we were incorrectly
setting it to TLS1.2 (current default level)
fixes: https://pagure.io/389-ds-base/issue/51060
Reviewed by: firstyear(Thanks!)
---
dirsrvtests/tests/suites/tls/ssl_version_test.py | 12 ++++++++++++
ldap/servers/slapd/ssl.c | 4 ++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/dirsrvtests/tests/suites/tls/ssl_version_test.py b/dirsrvtests/tests/suites/tls/ssl_version_test.py
index acc8b23..d9dae5a 100644
--- a/dirsrvtests/tests/suites/tls/ssl_version_test.py
+++ b/dirsrvtests/tests/suites/tls/ssl_version_test.py
@@ -19,10 +19,12 @@ def test_ssl_version_range(topo):
1. Get current default range
2. Set sslVersionMin and verify it is applied after a restart
3. Set sslVersionMax and verify it is applied after a restart
+ 4. Sanity test all the min/max versions
:expectedresults:
1. Success
2. Success
3. Success
+ 4. Success
"""
topo.standalone.enable_tls()
@@ -47,6 +49,16 @@ def test_ssl_version_range(topo):
max = enc.get_attr_val_utf8('sslVersionMax')
assert max == default_min
+ # Sanity test all the min/max versions
+ for attr, versions in [('sslVersionMin', ['TLS1.0', 'TLS1.1', 'TLS1.2', 'TLS1.0']),
+ ('sslVersionMax', ['TLS1.0', 'TLS1.1', 'TLS1.2'])]:
+ for version in versions:
+ # Test that the setting is correctly applied after a restart
+ enc.replace(attr, version)
+ topo.standalone.restart()
+ current_val = enc.get_attr_val_utf8(attr)
+ assert current_val == version
+
if __name__ == '__main__':
# Run isolated
diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c
index 9296cd4..0248585 100644
--- a/ldap/servers/slapd/ssl.c
+++ b/ldap/servers/slapd/ssl.c
@@ -1291,7 +1291,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emin);
(*rval) = enabledNSSVersions.min;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
} else {
if (enabledNSSVersions.max < CURRENT_DEFAULT_SSL_VERSION) {
@@ -1302,7 +1302,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emax);
(*rval) = enabledNSSVersions.max;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
}
} else if (tlsv < 1.2f) { /* TLS1.1 */
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 11 months
[389-ds-base] branch 389-ds-base-1.4.3 updated: Issue 51060 - unable to set sslVersionMin to TLS1.0
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
new a514aba Issue 51060 - unable to set sslVersionMin to TLS1.0
a514aba is described below
commit a514aba497a5cb756b104d23d8412945e24f2958
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Wed Apr 29 16:07:38 2020 -0400
Issue 51060 - unable to set sslVersionMin to TLS1.0
Description: When processing the "sslVersionMin" attribute we were incorrectly
setting it to TLS1.2 (current default level)
fixes: https://pagure.io/389-ds-base/issue/51060
Reviewed by: firstyear(Thanks!)
---
dirsrvtests/tests/suites/tls/ssl_version_test.py | 12 ++++++++++++
ldap/servers/slapd/ssl.c | 4 ++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/dirsrvtests/tests/suites/tls/ssl_version_test.py b/dirsrvtests/tests/suites/tls/ssl_version_test.py
index acc8b23..d9dae5a 100644
--- a/dirsrvtests/tests/suites/tls/ssl_version_test.py
+++ b/dirsrvtests/tests/suites/tls/ssl_version_test.py
@@ -19,10 +19,12 @@ def test_ssl_version_range(topo):
1. Get current default range
2. Set sslVersionMin and verify it is applied after a restart
3. Set sslVersionMax and verify it is applied after a restart
+ 4. Sanity test all the min/max versions
:expectedresults:
1. Success
2. Success
3. Success
+ 4. Success
"""
topo.standalone.enable_tls()
@@ -47,6 +49,16 @@ def test_ssl_version_range(topo):
max = enc.get_attr_val_utf8('sslVersionMax')
assert max == default_min
+ # Sanity test all the min/max versions
+ for attr, versions in [('sslVersionMin', ['TLS1.0', 'TLS1.1', 'TLS1.2', 'TLS1.0']),
+ ('sslVersionMax', ['TLS1.0', 'TLS1.1', 'TLS1.2'])]:
+ for version in versions:
+ # Test that the setting is correctly applied after a restart
+ enc.replace(attr, version)
+ topo.standalone.restart()
+ current_val = enc.get_attr_val_utf8(attr)
+ assert current_val == version
+
if __name__ == '__main__':
# Run isolated
diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c
index 2d7ba3e..846106b 100644
--- a/ldap/servers/slapd/ssl.c
+++ b/ldap/servers/slapd/ssl.c
@@ -1373,7 +1373,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emin);
(*rval) = enabledNSSVersions.min;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
} else {
if (enabledNSSVersions.max < CURRENT_DEFAULT_SSL_VERSION) {
@@ -1384,7 +1384,7 @@ set_NSS_version(char *val, PRUint16 *rval, int ismin)
val, emax);
(*rval) = enabledNSSVersions.max;
} else {
- (*rval) = CURRENT_DEFAULT_SSL_VERSION;
+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
}
}
} else if (tlsv < 1.2f) { /* TLS1.1 */
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 11 months