d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lib/libc/net/nsdispatch.c: Fix missing unlock and add locking annotations

Browse Source 
The error cases (goto fin) of _nsdispatch were missing the unlock.

This change also drops the checks for __isthreaded since the pthread stubs
are already no-ops if threads are not being used. Dropping those conditionals
allows clang's thread safety analysis to deal with the file and also makes
the code a bit more readable. While touching the file also add a few more
assertions in debug mode that the right locks are held.

Reviewed By:	markj
Differential Revision: https://reviews.freebsd.org/D29372
This commit is contained in:
Alex Richardson 2021-03-25 11:22:10 +00:00
parent 142cb88bc6
commit 5245bf7b92
1 changed files with 129 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

201
lib/libc/net/nsdispatch.c
View File

@ -69,6 +69,7 @@ __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <assert.h>
#include <dlfcn.h> #include <dlfcn.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
@ -76,6 +77,7 @@ __FBSDID("$FreeBSD$");
#include <nsswitch.h> #include <nsswitch.h>
#include <pthread.h> #include <pthread.h>
#include <pthread_np.h> #include <pthread_np.h>
#include <stdatomic.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
@ -97,7 +99,52 @@ enum _nss_constants {
* Global NSS data structures are mostly read-only, but we update * Global NSS data structures are mostly read-only, but we update
* them when we read or re-read the nsswitch.conf. * them when we read or re-read the nsswitch.conf.
*/ */
static pthread_rwlock_t nss_lock = PTHREAD_RWLOCK_INITIALIZER; static pthread_rwlock_t nss_lock = PTHREAD_RWLOCK_INITIALIZER;
#ifndef NDEBUG
static void *nss_wlock_owner __guarded_by(nss_lock);
#endif
static inline /* __lock_annotate(locks_excluded(nss_lock)) */
__locks_exclusive(nss_lock) int
nss_wlock(void)
{
int err;
err = _pthread_rwlock_wrlock(&nss_lock);
#ifndef NDEBUG
assert(nss_wlock_owner == NULL);
nss_wlock_owner = _pthread_self();
#endif
assert(err == 0 && "Locking should not have failed!");
return (err);
}
/*
* XXX: The manpage says already held lock may result in EDEADLK, but
* actually libthr returns always EBUSY, so we need the extra owner
* variable for assertions.
*/
#define ASSERT_NSS_WLOCK_HELD() \
do { \
if (__isthreaded) { \
assert(_pthread_rwlock_trywrlock(&nss_lock) == EBUSY); \
assert(nss_wlock_owner == _pthread_self()); \
} \
} while (0)
static inline __requires_exclusive(nss_lock) __unlocks(nss_lock) int
nss_wunlock(void)
{
int err;
ASSERT_NSS_WLOCK_HELD();
#ifndef NDEBUG
nss_wlock_owner = NULL;
#endif
err = _pthread_rwlock_unlock(&nss_lock);
assert(err == 0 && "Unlocking should not have failed!");
return (err);
}
/* /*
* Runtime determination of whether we are dynamically linked or not. * Runtime determination of whether we are dynamically linked or not.
@ -114,12 +161,12 @@ const ns_src __nsdefaultsrc[] = {
}; };
/* Database, source mappings. */ /* Database, source mappings. */
static unsigned int _nsmapsize; static unsigned int _nsmapsize __guarded_by(nss_lock);
static ns_dbt *_nsmap = NULL; static ns_dbt *_nsmap __guarded_by(nss_lock);
/* NSS modules. */ /* NSS modules. */
static unsigned int _nsmodsize; static unsigned int _nsmodsize __guarded_by(nss_lock);
static ns_mod *_nsmod; static ns_mod *_nsmod __guarded_by(nss_lock);
/* Placeholder for builtin modules' dlopen `handle'. */ /* Placeholder for builtin modules' dlopen `handle'. */
static int __nss_builtin_handle; static int __nss_builtin_handle;
@ -166,8 +213,7 @@ typedef int (*vector_comparison)(const void *, const void *);
typedef void (*vector_free_elem)(void *); typedef void (*vector_free_elem)(void *);
static void vector_sort(void *, unsigned int, size_t, static void vector_sort(void *, unsigned int, size_t,
vector_comparison); vector_comparison);
static void vector_free(void *, unsigned int *, size_t, static void _vector_free(void *, unsigned int, size_t, vector_free_elem);
vector_free_elem);
static void *vector_ref(unsigned int, void *, unsigned int, size_t); static void *vector_ref(unsigned int, void *, unsigned int, size_t);
static void *vector_search(const void *, void *, unsigned int, size_t, static void *vector_search(const void *, void *, unsigned int, size_t,
vector_comparison); vector_comparison);
@ -238,22 +284,24 @@ vector_ref(unsigned int i, void *vec, unsigned int count, size_t esize)
} }
#define VECTOR_FREE(v, c, s, f) \ #define VECTOR_FREE(vec, count, es, fe) do { \
do { vector_free(v, c, s, f); v = NULL; } while (0) _vector_free(vec, count, es, fe); \
vec = NULL; \
count = 0; \
} while (0)
static void static void
vector_free(void *vec, unsigned int *count, size_t esize, _vector_free(void *vec, unsigned int count, size_t esize,
vector_free_elem free_elem) vector_free_elem free_elem)
{ {
unsigned int i; unsigned int i;
void *elem; void *elem;
for (i = 0; i < *count; i++) { for (i = 0; i < count; i++) {
elem = vector_ref(i, vec, *count, esize); elem = vector_ref(i, vec, count, esize);
if (elem != NULL) if (elem != NULL)
free_elem(elem); free_elem(elem);
} }
free(vec); free(vec);
*count = 0;
} }
/* /*
@ -282,13 +330,14 @@ mtab_compare(const void *a, const void *b)
/* /*
* NSS nsmap management. * NSS nsmap management.
*/ */
void __requires_exclusive(nss_lock) void
_nsdbtaddsrc(ns_dbt *dbt, const ns_src *src) _nsdbtaddsrc(ns_dbt *dbt, const ns_src *src)
{ {
const ns_mod *modp; const ns_mod *modp;
dbt->srclist = vector_append(src, dbt->srclist, &dbt->srclistsize, ASSERT_NSS_WLOCK_HELD();
sizeof(*src)); dbt->srclist = vector_append(src, dbt->srclist,
(unsigned int *)&dbt->srclistsize, sizeof(*src));
modp = vector_search(&src->name, _nsmod, _nsmodsize, sizeof(*_nsmod), modp = vector_search(&src->name, _nsmod, _nsmodsize, sizeof(*_nsmod),
string_compare); string_compare);
if (modp == NULL) if (modp == NULL)
@ -325,26 +374,28 @@ _nsdbtdump(const ns_dbt *dbt)
} }
#endif #endif
#ifndef NS_REREAD_CONF
static int __guarded_by(nss_lock) already_initialized = 0;
#endif
/* /*
* The first time nsdispatch is called (during a process's lifetime, * The first time nsdispatch is called (during a process's lifetime,
* or after nsswitch.conf has been updated), nss_configure will * or after nsswitch.conf has been updated), nss_configure will
* prepare global data needed by NSS. * prepare global data needed by NSS.
*/ */
static int static __requires_shared(nss_lock) int
nss_configure(void) __lock_annotate(no_thread_safety_analysis) /* RWLock upgrade not supported. */
do_nss_configure(void)
{ {
static time_t confmod; static time_t __guarded_by(nss_lock) confmod = 0;
static int already_initialized = 0;
struct stat statbuf; struct stat statbuf;
int result, isthreaded; int result;
const char *path; const char *path;
#ifdef NS_CACHING #ifdef NS_CACHING
void *handle; void *handle;
#endif #endif
result = 0; result = 0;
isthreaded = __isthreaded;
#if defined(_NSS_DEBUG) && defined(_NSS_SHOOT_FOOT) #if defined(_NSS_DEBUG) && defined(_NSS_SHOOT_FOOT)
/* NOTE WELL: THIS IS A SECURITY HOLE. This must only be built /* NOTE WELL: THIS IS A SECURITY HOLE. This must only be built
* for debugging purposes and MUST NEVER be used in production. * for debugging purposes and MUST NEVER be used in production.
@ -353,36 +404,33 @@ nss_configure(void)
if (path == NULL) if (path == NULL)
#endif #endif
path = _PATH_NS_CONF; path = _PATH_NS_CONF;
result = _pthread_rwlock_unlock(&nss_lock);
if (result != 0)
return (result);
result = nss_wlock();
if (result != 0)
return (result);
#ifndef NS_REREAD_CONF #ifndef NS_REREAD_CONF
/* /*
* Define NS_REREAD_CONF to have nsswitch notice changes * Another thread could have already run the initialization
* to nsswitch.conf(5) during runtime. This involves calling * logic while we were waiting for the write lock. Check
* stat(2) every time, which can result in performance hit. * already_initialized to avoid re-initializing.
*/ */
if (already_initialized) if (already_initialized)
return (0); goto fin;
already_initialized = 1;
#endif /* NS_REREAD_CONF */ #endif /* NS_REREAD_CONF */
ASSERT_NSS_WLOCK_HELD();
if (stat(path, &statbuf) != 0) if (stat(path, &statbuf) != 0)
return (0); goto fin;
if (statbuf.st_mtime <= confmod) if (statbuf.st_mtime <= confmod)
return (0); goto fin;
if (isthreaded) {
(void)_pthread_rwlock_unlock(&nss_lock);
result = _pthread_rwlock_wrlock(&nss_lock);
if (result != 0)
return (result);
if (stat(path, &statbuf) != 0)
goto fin;
if (statbuf.st_mtime <= confmod)
goto fin;
}
_nsyyin = fopen(path, "re"); _nsyyin = fopen(path, "re");
if (_nsyyin == NULL) if (_nsyyin == NULL)
goto fin; goto fin;
VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap), VECTOR_FREE(_nsmap, _nsmapsize, sizeof(*_nsmap),
(vector_free_elem)ns_dbt_free); (vector_free_elem)ns_dbt_free);
VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod), VECTOR_FREE(_nsmod, _nsmodsize, sizeof(*_nsmod),
(vector_free_elem)ns_mod_free); (vector_free_elem)ns_mod_free);
if (confmod == 0) if (confmod == 0)
(void)atexit(nss_atexit); (void)atexit(nss_atexit);
@ -400,22 +448,42 @@ nss_configure(void)
dlclose(handle); dlclose(handle);
} }
#endif #endif
#ifndef NS_REREAD_CONF
already_initialized = 1;
#endif
fin: fin:
if (isthreaded) { result = nss_wunlock();
(void)_pthread_rwlock_unlock(&nss_lock); if (result == 0)
if (result == 0) result = _pthread_rwlock_rdlock(&nss_lock);
result = _pthread_rwlock_rdlock(&nss_lock); return (result);
} }
static __requires_shared(nss_lock) int
nss_configure(void)
{
int result;
#ifndef NS_REREAD_CONF
/*
* Define NS_REREAD_CONF to have nsswitch notice changes
* to nsswitch.conf(5) during runtime. This involves calling
* stat(2) every time, which can result in performance hit.
*/
if (already_initialized)
return (0);
#endif /* NS_REREAD_CONF */
result = do_nss_configure();
return (result); return (result);
} }
void __requires_exclusive(nss_lock) void
_nsdbtput(const ns_dbt *dbt) _nsdbtput(const ns_dbt *dbt)
{ {
unsigned int i; unsigned int i;
ns_dbt *p; ns_dbt *p;
ASSERT_NSS_WLOCK_HELD();
for (i = 0; i < _nsmapsize; i++) { for (i = 0; i < _nsmapsize; i++) {
p = vector_ref(i, _nsmap, _nsmapsize, sizeof(*_nsmap)); p = vector_ref(i, _nsmap, _nsmapsize, sizeof(*_nsmap));
if (string_compare(&dbt->name, &p->name) == 0) { if (string_compare(&dbt->name, &p->name) == 0) {
@ -478,13 +546,15 @@ nss_load_builtin_modules(void)
* argument is non-NULL, assume a built-in module and use reg_fn to * argument is non-NULL, assume a built-in module and use reg_fn to
* register it. Otherwise, search for a dynamic NSS module. * register it. Otherwise, search for a dynamic NSS module.
*/ */
static void static __requires_exclusive(nss_lock) void
nss_load_module(const char *source, nss_module_register_fn reg_fn) nss_load_module(const char *source, nss_module_register_fn reg_fn)
{ {
char buf[PATH_MAX]; char buf[PATH_MAX];
ns_mod mod; ns_mod mod;
nss_module_register_fn fn; nss_module_register_fn fn;
ASSERT_NSS_WLOCK_HELD();
memset(&mod, 0, sizeof(mod)); memset(&mod, 0, sizeof(mod));
mod.name = strdup(source); mod.name = strdup(source);
if (mod.name == NULL) { if (mod.name == NULL) {
@ -548,9 +618,9 @@ nss_load_module(const char *source, nss_module_register_fn reg_fn)
vector_sort(_nsmod, _nsmodsize, sizeof(*_nsmod), string_compare); vector_sort(_nsmod, _nsmodsize, sizeof(*_nsmod), string_compare);
} }
static int exiting = 0; static int exiting __guarded_by(nss_lock) = 0;
static void static __requires_exclusive(nss_lock) void
ns_mod_free(ns_mod *mod) ns_mod_free(ns_mod *mod)
{ {
@ -569,24 +639,19 @@ ns_mod_free(ns_mod *mod)
static void static void
nss_atexit(void) nss_atexit(void)
{ {
int isthreaded; (void)nss_wlock();
exiting = 1; exiting = 1;
isthreaded = __isthreaded; VECTOR_FREE(_nsmap, _nsmapsize, sizeof(*_nsmap),
if (isthreaded)
(void)_pthread_rwlock_wrlock(&nss_lock);
VECTOR_FREE(_nsmap, &_nsmapsize, sizeof(*_nsmap),
(vector_free_elem)ns_dbt_free); (vector_free_elem)ns_dbt_free);
VECTOR_FREE(_nsmod, &_nsmodsize, sizeof(*_nsmod), VECTOR_FREE(_nsmod, _nsmapsize, sizeof(*_nsmod),
(vector_free_elem)ns_mod_free); (vector_free_elem)ns_mod_free);
if (isthreaded) (void)nss_wunlock();
(void)_pthread_rwlock_unlock(&nss_lock);
} }
/* /*
* Finally, the actual implementation. * Finally, the actual implementation.
*/ */
static nss_method static __requires_shared(nss_lock) nss_method
nss_method_lookup(const char *source, const char *database, nss_method_lookup(const char *source, const char *database,
const char *method, const ns_dtab disp_tab[], void **mdata) const char *method, const ns_dtab disp_tab[], void **mdata)
{ {
@ -625,7 +690,7 @@ fb_endstate(void *p)
__weak_reference(_nsdispatch, nsdispatch); __weak_reference(_nsdispatch, nsdispatch);
int __requires_unlocked(nss_lock) int
_nsdispatch(void *retval, const ns_dtab disp_tab[], const char *database, _nsdispatch(void *retval, const ns_dtab disp_tab[], const char *database,
const char *method_name, const ns_src defaults[], ...) const char *method_name, const ns_src defaults[], ...)
{ {
@ -634,7 +699,7 @@ _nsdispatch(void *retval, const ns_dtab disp_tab[], const char *database,
const ns_src *srclist; const ns_src *srclist;
nss_method method, fb_method; nss_method method, fb_method;
void *mdata; void *mdata;
int isthreaded, serrno, i, result, srclistsize; int serrno, i, result, srclistsize;
struct fb_state *st; struct fb_state *st;
int saved_depth; int saved_depth;
@ -647,15 +712,8 @@ _nsdispatch(void *retval, const ns_dtab disp_tab[], const char *database,
dbt = NULL; dbt = NULL;
fb_method = NULL; fb_method = NULL;
isthreaded = __isthreaded;
serrno = errno; serrno = errno;
if (isthreaded) { (void)_pthread_rwlock_rdlock(&nss_lock);
result = _pthread_rwlock_rdlock(&nss_lock);
if (result != 0) {
result = NS_UNAVAIL;
goto fin;
}
}
result = fb_getstate(&st); result = fb_getstate(&st);
if (result != 0) { if (result != 0) {
@ -774,10 +832,9 @@ _nsdispatch(void *retval, const ns_dtab disp_tab[], const char *database,
} }
#endif /* NS_CACHING */ #endif /* NS_CACHING */
if (isthreaded)
(void)_pthread_rwlock_unlock(&nss_lock);
--st->dispatch_depth; --st->dispatch_depth;
fin: fin:
(void)_pthread_rwlock_unlock(&nss_lock);
errno = serrno; errno = serrno;
return (result); return (result);
} }