Re: nss_ldap broken

On Fri, 2004-03-26 at 14:51, Daniel Eischen wrote:
> On Fri, 26 Mar 2004, Sean McNeil wrote:
> 
> > OK, I think I understand this problem...
> > 
> > When I have my nsswitch.conf setup as follows, I get seg11s:
> > 
> > passwd: files ldap
> > group: files ldap
> > 
> > This appears to be an issue with any external nss_*.so.1 module that
> > uses pthread.  It looks to me it is about the following:
> > 
> > /*
> >  * Cleanup
> >  */
> > static void
> > nss_atexit(void)
> > {
> > 	(void)_pthread_rwlock_wrlock(&nss_lock);
> > 	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
> > 	    (vector_free_elem)ns_dbt_free);
> > 	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
> > 	    (vector_free_elem)ns_mod_free);
> > 	(void)_pthread_rwlock_unlock(&nss_lock);
> > }
> > 
> > In my case, the nss_ldap.so.1 module was loaded which pulls in
> > libpthread.  I'm not sure how this works without a libpthred, but it
> > would appear that unless libpthread.so is loaded everything is OK.  But
> > now, it has been loaded and the rwlock_wrlock() works, but then it has
> > been unloaded before rwlock_unlock() gets called.
> > 
> > Would using
> > 
> > #include <reentrant.h>
> > rwlock_wrlock()
> > rwlock_unlock()
> > 
> > macros fix this?
> 
> I think I made a comment about how you should always
> prefix _pthread_foo() calls with 'if (__isthreaded)'.
> 
> When the thread libraries are initialized, then overrwrite
> the function pointers in libc's thread jumptable.  If you
> unload the library, libc still retains those pointers.

Yes, checking __isthreaded solves my problem.  nsdispatch.c has several
calls to _thread* routines that should be protected by the __isthreaded
test.

It would appear that even though RTLD_LOCAL is used on the dlopen, since
nss_ldap.so.1 is linked with libc as well it causes those thread
jumptable entries to be modified.  When dlclose is called the reference
count to the thread library goes to zero, the library is unloaded, and
the pointers are now no good.

This poses an interesting problem, though.  Since there is a dlopen in
this it is possible for part of a function to have __isthreaded not set
and part of it set.  For these routines the value of __isthreaded would
need to be stashed.  Yet for the nss_atexit, since it is the opposite
direction (dlclose), the variable would have to be tested directly.

Here is my cut at solving the problem:

*** nsdispatch.c-orig	Mon Mar 15 00:14:35 2004
--- nsdispatch.c	Fri Mar 26 15:34:47 2004
*************** nss_configure(void)
*** 316,323 ****
  	static pthread_mutex_t conf_lock = PTHREAD_MUTEX_INITIALIZER;
  	static time_t	 confmod;
  	struct stat	 statbuf;
! 	int		 result;
  	const char	*path;
  
  #if defined(_NSS_DEBUG) && defined(_NSS_SHOOT_FOOT)
  	/* NOTE WELL:  THIS IS A SECURITY HOLE. This must only be built
--- 316,324 ----
  	static pthread_mutex_t conf_lock = PTHREAD_MUTEX_INITIALIZER;
  	static time_t	 confmod;
  	struct stat	 statbuf;
! 	int		 result = 0;
  	const char	*path;
+ 	int		isthreaded = __isthreaded;
  
  #if defined(_NSS_DEBUG) && defined(_NSS_SHOOT_FOOT)
  	/* NOTE WELL:  THIS IS A SECURITY HOLE. This must only be built
*************** nss_configure(void)
*** 331,343 ****
  		return (0);
  	if (statbuf.st_mtime <= confmod)
  		return (0);
! 	result = _pthread_mutex_trylock(&conf_lock);
! 	if (result != 0)
! 		return (0);
! 	(void)_pthread_rwlock_unlock(&nss_lock);
! 	result = _pthread_rwlock_wrlock(&nss_lock);
! 	if (result != 0)
! 		goto fin2;
  	_nsyyin = fopen(path, "r");
  	if (_nsyyin == NULL)
  		goto fin;
--- 332,346 ----
  		return (0);
  	if (statbuf.st_mtime <= confmod)
  		return (0);
! 	if (isthreaded) {
! 		result = _pthread_mutex_trylock(&conf_lock);
! 		if (result != 0)
! 			return (0);
! 		(void)_pthread_rwlock_unlock(&nss_lock);
! 		result = _pthread_rwlock_wrlock(&nss_lock);
! 		if (result != 0)
! 			goto fin2;
! 	}
  	_nsyyin = fopen(path, "r");
  	if (_nsyyin == NULL)
  		goto fin;
*************** nss_configure(void)
*** 353,362 ****
  		(void)atexit(nss_atexit);
  	confmod = statbuf.st_mtime;
  fin:
! 	(void)_pthread_rwlock_unlock(&nss_lock);
! 	result = _pthread_rwlock_rdlock(&nss_lock);
  fin2:
! 	(void)_pthread_mutex_unlock(&conf_lock);
  	return (result);
  }
  
--- 356,368 ----
  		(void)atexit(nss_atexit);
  	confmod = statbuf.st_mtime;
  fin:
! 	if (isthreaded) {
! 		(void)_pthread_rwlock_unlock(&nss_lock);
! 		result = _pthread_rwlock_rdlock(&nss_lock);
! 	}
  fin2:
! 	if (isthreaded)
! 		(void)_pthread_mutex_unlock(&conf_lock);
  	return (result);
  }
  
*************** ns_mod_free(ns_mod *mod)
*** 510,521 ****
  static void
  nss_atexit(void)
  {
! 	(void)_pthread_rwlock_wrlock(&nss_lock);
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
  	    (vector_free_elem)ns_mod_free);
! 	(void)_pthread_rwlock_unlock(&nss_lock);
  }
  
  
--- 516,529 ----
  static void
  nss_atexit(void)
  {
! 	if (__isthreaded)
! 		(void)_pthread_rwlock_wrlock(&nss_lock);
  	VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
  	    (vector_free_elem)ns_dbt_free);
  	VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod),
  	    (vector_free_elem)ns_mod_free);
! 	if (__isthreaded)
! 		(void)_pthread_rwlock_unlock(&nss_lock);
  }
  
  
*************** _nsdispatch(void *retval, const ns_dtab 
*** 569,580 ****
  	nss_method	 method;
  	void		*mdata;
  	int		 serrno, i, result, srclistsize;
  
  	serrno = errno;
! 	result = _pthread_rwlock_rdlock(&nss_lock);
! 	if (result != 0) {
! 		result = NS_UNAVAIL;
! 		goto fin;
  	}
  	result = nss_configure();
  	if (result != 0) {
--- 577,591 ----
  	nss_method	 method;
  	void		*mdata;
  	int		 serrno, i, result, srclistsize;
+ 	int		isthreaded = __isthreaded;
  
  	serrno = errno;
! 	if (isthreaded) {
! 		result = _pthread_rwlock_rdlock(&nss_lock);
! 		if (result != 0) {
! 			result = NS_UNAVAIL;
! 			goto fin;
! 		}
  	}
  	result = nss_configure();
  	if (result != 0) {
*************** _nsdispatch(void *retval, const ns_dtab 
*** 604,610 ****
  				break;
  		}
  	}
! 	(void)_pthread_rwlock_unlock(&nss_lock);
  fin:
  	errno = serrno;
  	return (result);
--- 615,622 ----
  				break;
  		}
  	}
! 	if (isthreaded)
! 		(void)_pthread_rwlock_unlock(&nss_lock);
  fin:
  	errno = serrno;
  	return (result);


Thanks,
Sean