ldap/servers/slapd/computed.c | 162 ++++++++++++++++++++++-----
ldap/servers/slapd/entrywsi.c | 2
ldap/servers/slapd/main.c | 3
ldap/servers/slapd/proto-slap.h | 1
ldap/servers/slapd/slapi-plugin.h | 1
ldap/servers/slapd/tools/rsearch/nametable.c | 4
6 files changed, 140 insertions(+), 33 deletions(-)
New commits:
commit aecddfc58a9b3d73ff831cacd56961a2554511ca
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Mon Jul 28 09:42:43 2014 -0700
Bug 1123477 - unauthenticated information disclosure
Fix Description: nscpentrywsi is returned only authenticated as root.
The bug was fixed by lkrispen(a)redhat.com (Ludwig Krispenz).
His patch was modified based upon this review comment.
https://bugzilla.redhat.com/show_bug.cgi?id=1123477#c2
https://bugzilla.redhat.com/show_bug.cgi?id=1123861
(cherry picked from commit aa90e26d5c4ea47b2a4a22f99cf0742cf48b3fae)
(cherry picked from commit 394277fdcef70078b54a280de88ab06dd289cc7a)
(cherry picked from commit fbb9bf0b37fdaec0856b9c78373a0fb1fa07a1dd)
(cherry picked from commit 5c2848665791adf25f884fe942f310392fd88162)
diff --git a/ldap/servers/slapd/computed.c b/ldap/servers/slapd/computed.c
index 7c99b45..7a80c96 100644
--- a/ldap/servers/slapd/computed.c
+++ b/ldap/servers/slapd/computed.c
@@ -59,6 +59,7 @@ struct _computed_attr_context {
struct _compute_evaluator {
struct _compute_evaluator *next;
slapi_compute_callback_t function;
+ int rootonly;
};
typedef struct _compute_evaluator compute_evaluator;
@@ -95,6 +96,13 @@ int compute_call_evaluators_nolock(computed_attr_context
*c,slapi_compute_output
compute_evaluator *current = NULL;
for (current = compute_evaluators; (current != NULL) && (-1 == rc);
current = current->next) {
+ if (current->rootonly) {
+ int isroot;
+ slapi_pblock_get(c->pb, SLAPI_REQUESTOR_ISROOT, &isroot);
+ if (!isroot) {
+ continue;
+ }
+ }
rc = (*(current->function))(c,type,e,outfn);
}
return rc;
@@ -157,14 +165,19 @@ compute_stock_evaluator(computed_attr_context *c,char*
type,Slapi_Entry *e,slapi
}
static void
-compute_add_evaluator_nolock(slapi_compute_callback_t function, compute_evaluator
*new_eval)
+compute_add_evaluator_nolock(slapi_compute_callback_t function, compute_evaluator
*new_eval, int rootonly)
{
new_eval->next = compute_evaluators;
new_eval->function = function;
+ new_eval->rootonly = rootonly;
compute_evaluators = new_eval;
}
int slapi_compute_add_evaluator(slapi_compute_callback_t function)
{
+ return slapi_compute_add_evaluator_ext(function, 0);
+}
+int slapi_compute_add_evaluator_ext(slapi_compute_callback_t function, int rootonly)
+{
int rc = 0;
compute_evaluator *new_eval = NULL;
PR_ASSERT(NULL != function);
@@ -187,7 +200,7 @@ int slapi_compute_add_evaluator(slapi_compute_callback_t function)
slapi_rwlock_wrlock(compute_evaluators_lock);
}
- compute_add_evaluator_nolock(function, new_eval);
+ compute_add_evaluator_nolock(function, new_eval, rootonly);
if (need_lock) {
slapi_rwlock_unlock(compute_evaluators_lock);
diff --git a/ldap/servers/slapd/entrywsi.c b/ldap/servers/slapd/entrywsi.c
index 919ff91..40fe39b 100644
--- a/ldap/servers/slapd/entrywsi.c
+++ b/ldap/servers/slapd/entrywsi.c
@@ -864,7 +864,7 @@ entry_compute_nscpentrywsi(computed_attr_context *c,char*
type,Slapi_Entry *e,sl
int
entry_computed_attr_init()
{
- slapi_compute_add_evaluator(entry_compute_nscpentrywsi);
+ slapi_compute_add_evaluator_ext(entry_compute_nscpentrywsi, 1 /* root only */);
return 0;
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 4e84060..702389a 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6014,6 +6014,7 @@ typedef int (*slapi_compute_output_t)(computed_attr_context
*c,Slapi_Attr *a , S
typedef int (*slapi_compute_callback_t)(computed_attr_context *c,char* type,Slapi_Entry
*e,slapi_compute_output_t outputfn);
typedef int (*slapi_search_rewrite_callback_t)(Slapi_PBlock *pb);
int slapi_compute_add_evaluator(slapi_compute_callback_t function);
+int slapi_compute_add_evaluator_ext(slapi_compute_callback_t function, int rootonly);
int slapi_compute_add_search_rewriter(slapi_search_rewrite_callback_t function);
int compute_rewrite_search_filter(Slapi_PBlock *pb);
commit 6e9e8f98db79b13af86b7a6881d883244d13476d
Author: Thierry bordaz (tbordaz) <tbordaz(a)redhat.com>
Date: Thu Mar 14 18:35:21 2013 +0100
Ticket 616 - High contention on computed attribute lock
Bug Description:
On search request, some requested attributes are computed:
- subschemasubentry
- nscpentrywsi
- numsubordinates
- hassubordinates
This is done with a linked list of registered callback.
When sending the results, each requested attribute is evaluated against all the
callbacks.
When going through the callbacks list the thread hold the lock on the list.
Currently all the server callbacks are registered at startup time with only
one thread registering them (main).
A custom plugin may register custom callback using slapi_compute_add_evaluator
The important point is that callback are only added to the list.
Also on standard deployment without custom plugin adding new computed attribute,
this list is built at startup.
The problem on computed attribute is that it can create contention between search
threads
trying to evaluate/compute the requested attributes
The same description applies to computed filters. For example with 'views'
or
filters containing "subordinates", the filter is recomputed.
This using a linked list of callbacks registered during startup.
A custom plugin may register custom callback using
slapi_compute_add_search_rewriter.
In short:
On standard deployment computed filters and computed attributes lists are unchanged
after startup.
Computed filters/attribute are only added to the lists.
Fix Description:
The fix consist to not protect the access to computed filter/attribute unless a
computed filter/attribute is added after startup.
The fix also contains a improvement of rsearch to support filters larger than 256
char
The fix also contains reworks according to review feedback.
Basically using a temporary variable to hold the actual value of require_*
The benefit is limited to 2% in terms of search throughput (8*4core machine)
https://fedorahosted.org/389/ticket/616
Reviewed by: Rich Megginson (thanks Rich !)
Platforms tested: 1*4cores fedora and 8*4cores RHEL6.4
Flag Day: no
Doc impact: no
(cherry picked from commit 4b7791f3ab9370ccf30b09077656e247682bf384)
(cherry picked from commit ff5a8770da1266d96f5e1b36b026d539cb55ec25)
(cherry picked from commit 7d4c51ed99704e6b8e8aab1b1d6a80157af48784)
diff --git a/ldap/servers/slapd/computed.c b/ldap/servers/slapd/computed.c
index b82e2e8..7c99b45 100644
--- a/ldap/servers/slapd/computed.c
+++ b/ldap/servers/slapd/computed.c
@@ -44,6 +44,7 @@
/* Handles computed attributes for entries as they're returned to the client */
#include "slap.h"
+#include "proto-slap.h"
/* Structure used to pass the context needed for completing a computed attribute
operation */
@@ -61,8 +62,11 @@ struct _compute_evaluator {
};
typedef struct _compute_evaluator compute_evaluator;
+static PRBool startup_completed = PR_FALSE;
+
static compute_evaluator *compute_evaluators = NULL;
static Slapi_RWLock *compute_evaluators_lock = NULL;
+static PRBool require_compute_evaluator_lock = PR_FALSE;
static int
compute_stock_evaluator(computed_attr_context *c,char* type,Slapi_Entry
*e,slapi_compute_output_t outputfn);
@@ -75,6 +79,7 @@ typedef struct _compute_rewriter compute_rewriter;
static compute_rewriter *compute_rewriters = NULL;
static Slapi_RWLock *compute_rewriters_lock = NULL;
+static PRBool require_compute_rewriters_lock = PR_FALSE;
/* Function called by evaluators to have the value output */
static int
@@ -83,17 +88,36 @@ compute_output_callback(computed_attr_context *c,Slapi_Attr *a ,
Slapi_Entry *e)
return encode_attr (c->pb, c->ber, e, a, c->attrsonly, c->requested_type);
}
+static
+int compute_call_evaluators_nolock(computed_attr_context *c,slapi_compute_output_t
outfn,char *type,Slapi_Entry *e)
+{
+ int rc = -1;
+ compute_evaluator *current = NULL;
+
+ for (current = compute_evaluators; (current != NULL) && (-1 == rc);
current = current->next) {
+ rc = (*(current->function))(c,type,e,outfn);
+ }
+ return rc;
+}
+
static int
compute_call_evaluators(computed_attr_context *c,slapi_compute_output_t outfn,char
*type,Slapi_Entry *e)
{
int rc = -1;
- compute_evaluator *current = NULL;
+ int need_lock = require_compute_evaluator_lock;
+
/* Walk along the list (locked) calling the evaluator functions util one says yes, an
error happens, or we finish */
- slapi_rwlock_rdlock(compute_evaluators_lock);
- for (current = compute_evaluators; (current != NULL) && (-1 == rc); current =
current->next) {
- rc = (*(current->function))(c,type,e,outfn);
- }
- slapi_rwlock_unlock(compute_evaluators_lock);
+
+ if (need_lock) {
+ slapi_rwlock_rdlock(compute_evaluators_lock);
+ }
+
+ rc = compute_call_evaluators_nolock(c, outfn, type, e);
+
+ if (need_lock) {
+ slapi_rwlock_unlock(compute_evaluators_lock);
+ }
+
return rc;
}
@@ -132,25 +156,54 @@ compute_stock_evaluator(computed_attr_context *c,char*
type,Slapi_Entry *e,slapi
return rc; /* I see no ships */
}
+static void
+compute_add_evaluator_nolock(slapi_compute_callback_t function, compute_evaluator
*new_eval)
+{
+ new_eval->next = compute_evaluators;
+ new_eval->function = function;
+ compute_evaluators = new_eval;
+}
int slapi_compute_add_evaluator(slapi_compute_callback_t function)
{
int rc = 0;
compute_evaluator *new_eval = NULL;
PR_ASSERT(NULL != function);
PR_ASSERT(NULL != compute_evaluators_lock);
- slapi_rwlock_wrlock(compute_evaluators_lock);
+ if (startup_completed) {
+ /* We are now in multi-threaded and we still add
+ * a attribute evaluator.
+ * switch to use locking mechanimsm
+ */
+ require_compute_evaluator_lock = PR_TRUE;
+ }
+
new_eval = (compute_evaluator *)slapi_ch_calloc(1,sizeof (compute_evaluator));
if (NULL == new_eval) {
rc = ENOMEM;
} else {
- new_eval->next = compute_evaluators;
- new_eval->function = function;
- compute_evaluators = new_eval;
+ int need_lock = require_compute_evaluator_lock;
+
+ if (need_lock) {
+ slapi_rwlock_wrlock(compute_evaluators_lock);
+ }
+
+ compute_add_evaluator_nolock(function, new_eval);
+
+ if (need_lock) {
+ slapi_rwlock_unlock(compute_evaluators_lock);
+ }
}
- slapi_rwlock_unlock(compute_evaluators_lock);
+
return rc;
}
+/* Called when */
+void
+compute_plugins_started()
+{
+ startup_completed = PR_TRUE;
+}
+
/* Call this on server startup, before the first LDAP operation is serviced */
int compute_init()
{
@@ -200,6 +253,14 @@ int compute_terminate()
return 0;
}
+static void
+compute_add_search_rewrite_nolock(slapi_search_rewrite_callback_t function,
compute_rewriter *new_rewriter)
+{
+ new_rewriter->next = compute_rewriters;
+ new_rewriter->function = function;
+ compute_rewriters = new_rewriter;
+}
+
/* Functions dealing with re-writing of search filters */
int slapi_compute_add_search_rewriter(slapi_search_rewrite_callback_t function)
@@ -208,36 +269,66 @@ int
slapi_compute_add_search_rewriter(slapi_search_rewrite_callback_t function)
compute_rewriter *new_rewriter = NULL;
PR_ASSERT(NULL != function);
PR_ASSERT(NULL != compute_rewriters_lock);
+ if (startup_completed) {
+ /* We are now in multi-threaded and we still add
+ * a filter rewriter.
+ * switch to use locking mechanimsm
+ */
+ require_compute_rewriters_lock = PR_TRUE;
+ }
new_rewriter = (compute_rewriter *)slapi_ch_calloc(1,sizeof (compute_rewriter));
if (NULL == new_rewriter) {
rc = ENOMEM;
} else {
- slapi_rwlock_wrlock(compute_rewriters_lock);
- new_rewriter->next = compute_rewriters;
- new_rewriter->function = function;
- compute_rewriters = new_rewriter;
- slapi_rwlock_unlock(compute_rewriters_lock);
+ int need_lock = require_compute_rewriters_lock;
+
+ if (need_lock) {
+ slapi_rwlock_wrlock(compute_rewriters_lock);
+ }
+
+ compute_add_search_rewrite_nolock(function, new_rewriter);
+
+ if (need_lock) {
+ slapi_rwlock_unlock(compute_rewriters_lock);
+ }
}
return rc;
}
-int compute_rewrite_search_filter(Slapi_PBlock *pb)
+static
+int compute_rewrite_search_filter_nolock(Slapi_PBlock *pb)
+{
+ int rc = -1;
+ compute_rewriter *current = NULL;
+
+ for (current = compute_rewriters; (current != NULL) && (-1 == rc);
current = current->next) {
+ rc = (*(current->function))(pb);
+ /* Meaning of the return code :
+ -1 : keep looking
+ 0 : rewrote OK
+ 1 : refuse to do this search
+ 2 : operations error
+ */
+ }
+ return(rc);
+}
+
+int compute_rewrite_search_filter(Slapi_PBlock *pb)
{
/* Iterate through the listed rewriters until one says it matched */
int rc = -1;
- compute_rewriter *current = NULL;
+ int need_lock = require_compute_rewriters_lock;
+
/* Walk along the list (locked) calling the evaluator functions util one says yes, an
error happens, or we finish */
- slapi_rwlock_rdlock(compute_rewriters_lock);
- for (current = compute_rewriters; (current != NULL) && (-1 == rc); current =
current->next) {
- rc = (*(current->function))(pb);
- /* Meaning of the return code :
- -1 : keep looking
- 0 : rewrote OK
- 1 : refuse to do this search
- 2 : operations error
- */
- }
- slapi_rwlock_unlock(compute_rewriters_lock);
+ if (need_lock) {
+ slapi_rwlock_rdlock(compute_rewriters_lock);
+ }
+
+ rc = compute_rewrite_search_filter_nolock(pb);
+
+ if (need_lock) {
+ slapi_rwlock_unlock(compute_rewriters_lock);
+ }
return rc;
}
diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c
index 737da5d..bc07cbb 100644
--- a/ldap/servers/slapd/main.c
+++ b/ldap/servers/slapd/main.c
@@ -1171,7 +1171,8 @@ main( int argc, char **argv)
slapi_td_dn_init();
plugin_print_lists();
- plugin_startall(argc, argv, 1 /* Start Backends */, 1 /* Start Globals */);
+ plugin_startall(argc, argv, 1 /* Start Backends */, 1 /* Start Globals */);
+ compute_plugins_started();
if (housekeeping_start((time_t)0, NULL) == NULL) {
return_value = 1;
goto cleanup;
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 1fb782b..76489ab 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -248,6 +248,7 @@ void do_compare( Slapi_PBlock *pb );
int compute_attribute(char *type, Slapi_PBlock *pb,BerElement *ber,Slapi_Entry *e,int
attrsonly,char *requested_type);
int compute_init();
int compute_terminate();
+void compute_plugins_started();
/*
diff --git a/ldap/servers/slapd/tools/rsearch/nametable.c
b/ldap/servers/slapd/tools/rsearch/nametable.c
index 0a4b6fc..e5d04cd 100644
--- a/ldap/servers/slapd/tools/rsearch/nametable.c
+++ b/ldap/servers/slapd/tools/rsearch/nametable.c
@@ -152,8 +152,8 @@ int nt_load(NameTable *nt, const char *filename)
if (!fd) return 0;
while (PR_Available(fd) > 0) {
- char temp[256], *s;
- if (PR_GetLine(fd, temp, 256)) break;
+ char temp[4096], *s;
+ if (PR_GetLine(fd, temp, sizeof(temp))) break;
s = strdup(temp);
if (!s) break;
if (!nt_push(nt, s)) break;