Re: (ITS#4980) misaligned memory access (openssl?)
by richton＠nbcs.rutgers.edu
I'm going to claim this as possible corruption because the RADIUS module
was definitely overstepping bounds when this happened...feel free to close
this out, I'll keep an eye out for recurrences.
On Wed, 23 May 2007, Howard Chu wrote:
> This stack trace is pretty exclusively OpenSSL and libc code. Unless there's
> been a heap corruption somewhere, it appears to be an OpenSSL issue.
16 years, 3 months
Re: (ITS#4951) [contrib] RADIUS password module fixes
by richton＠nbcs.rutgers.edu
I think this does it; I'm now happy with RADIUS in production.
--- ldap-src/contrib/slapd-modules/passwd/radius.c 2007-05-30 19:38:29.210866000 -0400
+++ radius.c 2007-05-29 11:02:09.728936000 -0400
@@ -17,6 +17,7 @@
#include <lber.h>
#include <lber_pvt.h> /* BER_BVC definition */
#include "lutil.h"
+#include <portable.h>
#include <ldap_pvt_thread.h>
#include <ac/string.h>
#include <ac/unistd.h>
On Tue, 29 May 2007, Aaron Richton wrote:
> Boy, that was some real quality time with dbx...ugh.
>
>
> I'm fairly confident that the issue is that portable.h isn't included and
> should have been as part of my patch. This worked in a development
> environment; I'll be putting it through some more tests and, most likely, in
> production soon (the =NULL workaround is just a time bomb).
16 years, 3 months
Re: (ITS#4972) thr_debug problems
by h.b.furuseth＠usit.uio.no
I wrote:
> RE23/thr_debug.c has some other problems that are fixed in HEAD, but
> take a bit more work to fix and test. I'll align it with HEAD if
> anyone is interested.
Apparenlty not. I'll import the fixes local to thr_debug.c to RE23,
but not the ones that need changes in include/.
--
Regards,
Hallvard
16 years, 3 months
(ITS#4988) --disable-overlays breaks overlays/Makefile
by h.b.furuseth＠usit.uio.no
Full_Name: Hallvard B Furuseth
Version: RE23
OS: HPUX
URL:
Submission from: (NULL) (129.240.202.105)
Submitted by: hallvard
'make' on HP-UX does not like '\' - newline - blank line, produced by
OBJS = overlays.o \
statover.o \
@SLAPD_STATIC_OVERLAYS@
in overlays/Makefile.in with --disable-overlays.
Fixed in HEAD overlays/Makefile.in rev 1.46, importing to RE23 now.
16 years, 3 months
Re: (ITS#4943) tpool.c pause vs. finish
by h.b.furuseth＠usit.uio.no
Some more comments, review, planned changes, and questions.
I guess I should summarize the bugs I've fixed in HEAD first, for
completeness. (I was a bit too busy at the time.) Restating the CVS
log comments, nothing new but maybe a bit clearer. Possibly some of the
issues are tpool-only issues and can't happen the way slapd uses tpool.
----------------------------------------
rev 1.63:
ldap_pvt_thread_pool_submit(), after failed thread creation:
- ltp_pending_count++ should be --.
- should signal if there are no more threads, in case pool_destroy()
is waiting for that. (Not sure if that can happen.)
ldap_int_thread_pool_wrapper():
- The 'if(ltp_state...)' was spurious: It would always be true, and
if it were not we'd get an eternal loop.
rev 1.64:
pool_pause():
- would cancel states FINISHING and STOPPING, pool_resume() would set
it back to RUNNING.
- could hang when waiting for another pool_pause(), since it did not
signal ltp_pcond if it decremented ltp_active_count to 1.
pool_wrapper():
- could update thread_keys[] during a pause while initializing and
possibly while finishing,
- just after initializing, could handle a new ctx during a pause.
pool_destroy():
- would go on with destruction if pool_resume() was called, since it
used an if(){wait} instead of while() and they use the same cond.
rev 1.65:
- pool_context() read thread_keys[] without mutex protection.
pool_purgekey() too, but it expected a pause to protect the keys.
pool_wrapper() protected with ltp_mutex while writing, and did not
quite ensure a pause.
rev 1.66:
- thread_keys[] is a (poor) open-addessed hash table, but it lacked
a "deleted item" mark. So keys (contexts) could become hidden and
re-emerge later if threads exited and then new ones were started.
- TID_HASH() did hash calculation in signed arithmetic, thus depending
on friendly integer overflow.
rev 1.67:
- setkey/getkey expected ldap_int_thread_userctx_t.ltu_key[] to be
NULL-terminated when not full. purgekey() could set a NULL in
the middle, hiding later keys until a new key overwrote the NULL.
API changes:
- setkey(data=NULL, kfree!=NULL) searched as if intended to reset
the key, but updated by setting the key. Now always updates.
- setkey(key=<not found>, data=NULL) could return either success or
failure. Now succeeds iff (data == NULL && kfree == NULL).
rev 1.68:
- ltp_max_count <= 0 meant allow an unlimited number of threads, but
thread #LDAP_MAXTHR+1 and later would loop forever looking for a
free thread_keys[] slot. (Not fixed for multi-pool applications.)
----------------------------------------
I wrote in the tpool.c 1.66 -> 1.67 patch:
> API changes, need review - I'm not sure what's indented here:
> - setkey(data=NULL, kfree!=NULL) searched as if intended to reset
> the key, but updated by setting the key. Now always updates.
Right. That matches the one place it can happen: In bdb_locker_id()
with setkey(,,data=(void *)(long)lockid,). I don't know if lockid
can be 0, but it doesn't need to for data to be NULL.
But it would cleaner to ch_malloc an integer instead of the (void*)
cast. Then it won't depend on integer->pointer->integer conversions
preserving the value either.
BTW, is there a reason for the cast to long? lockid is u_int32_t,
and the getkey 'long' result is restored to a u_int32_t.
Howard Chu wrote:
>> Also if ctx is invalid (another thread handled it) and a new
>> pending request has gotten the now-unused ctx, then the
>> 'if (pool->ltp_open_count == 0)' code will free that new
>> pending request instead of the old one.
>
> Most likely this is due to unlocking and relocking the mutex around
> the semaphore code. Axe that, remove the gratuitous unlock/relock,
> and then that problem goes away.
Was that an instruction/permission to axe it or not?
If not I'll just move the unlock/lock into the #ifdef for now.
> Thus far we've only had to worry about a single pool. If anyone can
> come up with a reason to need multiple pools, I guess we need to
> hear it.
I'll insert "static int initialized; assert(!initialized++);"
in pool_init() and see if anyone complains.
>> - setkey does not free the old data with ltk_free. Possibly that's
>> intentional, but if so that requires weird code elsewhere.
>
> Yes, that's intentional. ltk_free is only intended for cleanup if a key
> is still set by the time a thread exits. All of the keys we set are live
> for the entire life of a thread, they don't get re-assigned.
Even the (void*)slap_sasl_bind key?
lutil_passwd() looks like it can be recursive: Register a password
scheme which finds an entry in LDAP and authenticates via that
entry's userPassword - which again could call lutil_passwd().
If that's legal, this code in be_isroot_pw()/slap_passwd_check()
can break:
pool_setkey(, slap_sasl_bind, <ctx>, <freefunc> );
... /* slap_passwd_check() does access_allowed() here */
lutil_passwd();
...
pool_setkey(, slap_sasl_bind, NULL, NULL);
The final setkey would need to restore the old key.
I'm not sure of contrib/slapd-modules/smbk5pwd/smbk5pwd.c + some
overlay either. rwm + back-relay which uses another smbk5pwd or
something. (And I have no idea of the BDB cache stuff, but since
you wrote it I assume you know:-)
If it's a problem, we can replace code like that with
void *data = <ctx>;
ldap_pvt_thread_pool_keyfree_t *kfree = <freefunc>;
pool_swapkey(, slap_sasl_bind, &data, &kfree ); /* new function */
lutil_passwd();
pool_setkey(, slap_sasl_bind, data, kfree );
or if setkey reverts to ignoring kfree when it checks whether we are
setting or deleting a key, the kfree function can be passed directly.
I wrote:
> But this reminds me: How much should an ltk_free() function be allowed
> to do?
> - pool_context_reset() calls ltk_free(). Should pool_wrapper() go
> active (wait for !pause, increment ltp_active_count, unlock ltp_mutex)
> before calling pool_context_reset()?
I think so. It makes slapi safer, and I don't see that it can hurt:
context_reset() can call connection_fake_destroy(), which via
slapi_int_free_object_extensions() calls slapi module destructors,
which can do who-knows-what.
Another stray thought: It would similarly be safer for context_reset()
and purgekey() to delete the key before calling ltk_free, so that
function can't access the key's data after freeing it. OTOH that would
break any slapi module whose ltk_free calls a function which uses the
key before freeing it. I'll leave it alone unless someone says otherwise.
Other matters:
pool->ltp_active_list is maintained but not used for anything.
Do I delete it? Or does tpool maintain the list to ease debugging?
Use of tpool:
Another bug:
Slapd does not always check if pool_submit() and pool_setkey() succeed,
even though failure is not a bug. (They can run into some limits.)
I do not understand the calls to ldap_pvt_thread_pool_context_reset()
in slapd (init.c and bconfig.c). That frees the thread's sl_malloc
memory with slap_sl_mem_destroy(), which seems rather hazardous until
the thread is really, really done.
For that matter, I guess that means context_reset() had better release
keys in the reverse order of how they were set. That usually happened,
but not always, and my clear_key_idx() made the "not always" more
common. So to close the gap from a deleted key I'll make it move all
later keys forward, instead of just moving the latest key to the gap.
Bdb uses the database environment address as a key to pool_setkey().
So there can be at most ~30 BDB databases with an active key at the
same time in the same thread. What does that mean - 30 overlays
all using bdb? Access control groups spanning 30 databases?
Nitpick: Slapd uses both function pointers and data pointers as keys to
pool_[sg]etkey(). That makes it possible for two "different" keys to
compare equal, on a host where functions and data live in different
segments that otherwise share the same address space. So I guess I
should clean up and replace (void*)<function> keys with the address of
static char <function>_key;
--
Regards,
Hallvard
16 years, 3 months
Re: (ITS#4951) [contrib] RADIUS password module fixes
by richton＠nbcs.rutgers.edu
Boy, that was some real quality time with dbx...ugh.
I'm fairly confident that the issue is that portable.h isn't included and
should have been as part of my patch. This worked in a development
environment; I'll be putting it through some more tests and, most likely,
in production soon (the =NULL workaround is just a time bomb).
On Thu, 24 May 2007, Aaron Richton wrote:
> On Tue, 22 May 2007, Pierangelo Masarati wrote:
>> If that's an issue, I have no problem in explicitly setting that pointer
>> to NULL.
>
> I got some more dbx time on this, and it looks like that's a workaround for
> something that shouldn't be happening/needs to be fixed anyway.
> config_filename is always NULL at dlopen() time, just as it should be.
>
> Calling lock(&libradius_mutex) apparently overwrites config_filename, and
> explicitly initializing to NULL somehow stops this from happening. I can't
> see anything wrong with radius.c (as in HEAD), though; if anybody can weigh
> in on this, it'd be much appreciated. I suppose it could still be a compiler
> or a libthread bug, but I'm still missing a smoking gun here.
>
>
16 years, 3 months
Re: (ITS#4985) pthread.h: present but cannot be compiled
by maugustine＠phsyes.com
This is a multi-part message in MIME format.
------=_NextPart_000_0004_01C7A1C1.FB9C6800
Content-Type: text/plain;
charset="us-ascii"
Content-Transfer-Encoding: 7bit
For those of you that may have this same issue, I discovered that the
problem was with gcc version 4.0.0 that loads with AIX 5.3. After I
upgraded to 4.1.1, this problem went away.
------=_NextPart_000_0004_01C7A1C1.FB9C6800
Content-Type: text/html;
charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
<html xmlns:o=3D"urn:schemas-microsoft-com:office:office" =
xmlns:w=3D"urn:schemas-microsoft-com:office:word" =
xmlns=3D"http://www.w3.org/TR/REC-html40">
<head>
<META HTTP-EQUIV=3D"Content-Type" CONTENT=3D"text/html; =
charset=3Dus-ascii">
<meta name=3DGenerator content=3D"Microsoft Word 11 (filtered medium)">
<style>
<!--
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman";}
a:link, span.MsoHyperlink
{color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:Arial;
color:windowtext;}
@page Section1
{size:8.5in 11.0in;
margin:1.0in 1.25in 1.0in 1.25in;}
div.Section1
{page:Section1;}
-->
</style>
</head>
<body lang=3DEN-US link=3Dblue vlink=3Dpurple>
<div class=3DSection1>
<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>For those of you that may have this same issue, I =
discovered
that the problem was with gcc version 4.0.0 that loads with AIX =
5.3. After I
upgraded to 4.1.1, this problem went away.<o:p></o:p></span></font></p>
</div>
</body>
</html>
------=_NextPart_000_0004_01C7A1C1.FB9C6800--
16 years, 3 months
(ITS#4987) imporvement to slapo-constraint
by manu＠netbsd.org
Full_Name: Emmanuel Dreyfus
Version: 4.4.4
OS: NetBSD
URL: ftp://ftp.openldap.org/incoming/manu-070529.ext
Submission from: (NULL) (213.41.141.172)
I modified slapo-constraint so that it can verify that an attribute value
is bound to the existing values of another attribute. The purpose is to
make referential integrity easier to obtain.
Attached is a draft patch for review.
Then there is the UI question. For now it's configured in slapd.conf like
this:
database bdb
suffix "dc=example,dc=net"
overlay constraint
constraint_attribute title key netExampleTitle
Which means that add and modify on title will fail if the new value is
not an existing netExampleTitle value. I used the keyword "key" with
RDBMS referential integrity in mind, but I'm not sure it's that clear
in this context. Suggestions are welcome.
The update to man page is missing yet. I know.
Also, I had trouble understanding the style used in these sources. Is there
an official style guide for OpenLDAP?
16 years, 3 months
(ITS#4986) manageDSAit might control cause problems in database selection
by nvoutsin＠noc.uoa.gr
Full_Name: Nikos Voutsinas
Version: 2.3.35
OS: Solaris 9
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (195.134.100.207)
With the following configuration, search operations in the local database
("dc=foo,dc=bar") will fail when manageDSAit control is set.
....
....
backend bdb
database bdb
suffix "dc=foo,dc=bar"
rootdn "uid=manager,dc=foo,dc=bar"
rootpw "secret"
directory /var/openldap/bdb
access to * by * read
database relay
suffix ""
relay "dc=foo,dc=bar" massage
access to * by * read
....
....
The problem has been tracked down in select_backend().
16 years, 3 months
(ITS#4985) pthread.h: present but cannot be compiled
by maugustine＠phsyes.com
Full_Name: Matt Augustine
Version: 2.3.32
OS: AIX 5.3
URL:
Submission from: (NULL) (208.8.86.2)
Dear help,
I have currently installed openssl-0.9.8e and am looking to install
openldap-2.3.32 on AIX 5.3. However, when I go to run the configure script it
ends with the following warning:
pthread.h: present but cannot be compiled / check for missing prerequisite
headers? / see the Autoconf documentation secion "Present But cannot Be
Compiled" proceeing with the preprocessor's result / in the future, the compiler
will take precedence / Report this to <http://www.openldap.org/its/>
I then went to check the config.log and tied it to the following errors:
/usr/include/pthread.h:666: error: parse error before '*' token
/usr/include/pthread.h:669: error: parse error before '*' token
/usr/include/pthread.h:672: error: parse error before '*' token
/usr/include/pthread.h:675: error: parse error before '*' token
/usr/include/pthread.h:678: error: parse error before '*' token
/usr/include/pthread.h:686: error: parse error before '*' token
/usr/include/pthread.h:689: error: parse error before '*' token
/usr/include/pthread.h:692: error: parse error before '*' token
/usr/include/pthread.h:695: error: parse error before '*' token
/usr/include/pthread.h:703: error: parse error before '*' token
/usr/include/pthread.h:707: error: parse error before '*' token
/usr/include/pthread.h:710: error: parse error before '*' token
Those lines in pthread.h correspond to the following:
666: pthread_spin_init __((pthread_spinlock_t *, int));
669: pthread_spin_destroy __((pthread_spinlock_t *));
672: pthread_spin_lock __((pthread_spinlock_t *));
675: pthread_spin_unlock __((pthread_spinlock_t *));
678: pthread_spin_trylock __((pthread_spinlock_t *));
686: pthread_barrierattr_init __((pthread_barrierattr_t *));
689: pthread_barrierattr_destroy __((pthread_barrierattr_t *));
692: pthread_barrierattr_getpshared __((const pthread_barrierattr_t
*__restrict__,
int *__restrict__));
695: pthread_barrierattr_setpshared __((pthread_barrierattr_t *, int));
703: pthread_barrier_init __((pthread_barrier_t *__restrict__,
const pthread_barrierattr_t *__restrict__,
unsigned));
707: pthread_barrier_destroy __((pthread_barrier_t *));
710: pthread_barrier_wait __((pthread_barrier_t *));
Unfortunately, I'm fairly new to AIX, and building things from source, so these
lines really don't mean much to me. Any help that you can provide would be
greatly appreciated. Thanks!
16 years, 4 months
