* [RFC PATCH v5 0/3] adding pthread support to gssd
@ 2016-04-27 16:58 Olga Kornievskaia
2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Olga Kornievskaia @ 2016-04-27 16:58 UTC (permalink / raw)
To: steved; +Cc: linux-nfs
Adding support for multi-threaded gssd and allow for parallel upcalls.
For each upcall create a thread, detach to allow for automatic cleanup.
Communicate location of credential cache with gss api function instead
of an environmental variable. Use syscall api instead of libc setuid()
to change user identity.
Previously parent thread after detaching from child thread would call
yield to let the child thread run and consume the event from the fd.
Instead, use synchronization variables to signal parent after reading
from fd and have parent thread wait for the event.
Olga Kornievskaia (3):
gssd: use pthreads to handle upcalls
gssd: using syscalls directly to change thread's identity
gssd: always call gss_krb5_ccache_name
aclocal/libpthread.m4 | 13 ++++++++++
configure.ac | 3 +++
utils/gssd/Makefile.am | 3 ++-
utils/gssd/gssd.c | 44 ++++++++++++++++++++++++++++++---
utils/gssd/gssd.h | 5 ++++
utils/gssd/gssd_proc.c | 66 +++++++++++++++++++++++---------------------------
utils/gssd/krb5_util.c | 59 ++++++++++----------------------------------
utils/gssd/krb5_util.h | 1 -
8 files changed, 106 insertions(+), 88 deletions(-)
create mode 100644 aclocal/libpthread.m4
--
1.8.3.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-27 16:58 [RFC PATCH v5 0/3] adding pthread support to gssd Olga Kornievskaia @ 2016-04-27 16:58 ` Olga Kornievskaia 2016-04-27 17:39 ` Steve Dickson 2016-04-28 13:13 ` Jeff Layton 2016-04-27 16:58 ` [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity Olga Kornievskaia ` (2 subsequent siblings) 3 siblings, 2 replies; 16+ messages in thread From: Olga Kornievskaia @ 2016-04-27 16:58 UTC (permalink / raw) To: steved; +Cc: linux-nfs Currently, to persevere global data over multiple mounts, the root process does not fork when handling an upcall. Instead on not-forking create a pthread to handle the upcall since global data can be shared among threads. Signed-off-by: Steve Dickson <steved@redhat.com> Signed-off-by: Olga Kornievskaia <kolga@netapp.com> --- aclocal/libpthread.m4 | 13 +++++++++++++ configure.ac | 3 +++ utils/gssd/Makefile.am | 3 ++- utils/gssd/gssd.c | 45 ++++++++++++++++++++++++++++++++++++++++----- utils/gssd/gssd.h | 5 +++++ utils/gssd/gssd_proc.c | 49 ++++++++++++++++++------------------------------- utils/gssd/krb5_util.c | 3 +++ 7 files changed, 84 insertions(+), 37 deletions(-) create mode 100644 aclocal/libpthread.m4 diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 new file mode 100644 index 0000000..e87d2a0 --- /dev/null +++ b/aclocal/libpthread.m4 @@ -0,0 +1,13 @@ +dnl Checks for pthreads library and headers +dnl +AC_DEFUN([AC_LIBPTHREAD], [ + + dnl Check for library, but do not add -lpthreads to LIBS + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=-lpthread], + [AC_MSG_ERROR([libpthread not found.])]) + AC_SUBST(LIBPTHREAD) + + AC_CHECK_HEADERS([pthread.h], , + [AC_MSG_ERROR([libpthread headers not found.])]) + +])dnl diff --git a/configure.ac b/configure.ac index 25d2ba4..e0ecd61 100644 --- a/configure.ac +++ b/configure.ac @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to have KRBLIBS set AC_LIBRPCSECGSS + dnl Check for pthreads + AC_LIBPTHREAD + dnl librpcsecgss already has a dependency on libgssapi, dnl but we need to make sure we get the right version if test "$enable_gss" = yes; then diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am index cb040b3..3f5f59a 100644 --- a/utils/gssd/Makefile.am +++ b/utils/gssd/Makefile.am @@ -49,7 +49,8 @@ gssd_LDADD = \ $(RPCSECGSS_LIBS) \ $(KRBLIBS) \ $(GSSAPI_LIBS) \ - $(LIBTIRPC) + $(LIBTIRPC) \ + $(LIBPTHREAD) gssd_LDFLAGS = \ $(KRBLDFLAGS) diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c index e7cb07f..b676341 100644 --- a/utils/gssd/gssd.c +++ b/utils/gssd/gssd.c @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; char *preferred_realm = NULL; /* Avoid DNS reverse lookups on server names */ static bool avoid_dns = true; - +int thread_started = false; +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; TAILQ_HEAD(topdir_list_head, topdir) topdir_list; @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info *clp) static void gssd_scan(void); +static inline void wait_for_child_and_detach(pthread_t th) +{ + pthread_mutex_lock(&pmutex); + while (!thread_started) + pthread_cond_wait(&pcond, &pmutex); + thread_started = false; + pthread_mutex_unlock(&pmutex); + pthread_detach(th); +} + +/* For each upcall create a thread, detach from the main process so that + * resources are released back into the system without the need for a join. + * We need to wait for the child thread to start and consume the event from + * the file descriptor. + */ static void gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data) { struct clnt_info *clp = data; - - handle_gssd_upcall(clp); + pthread_t th; + int ret; + + ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall, + (void *)clp); + if (ret != 0) { + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", + ret, strerror(errno)); + return; + } + wait_for_child_and_detach(th); } static void gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void *data) { struct clnt_info *clp = data; - - handle_krb5_upcall(clp); + pthread_t th; + int ret; + + ret = pthread_create(&th, NULL, (void *)handle_krb5_upcall, + (void *)clp); + if (ret != 0) { + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", + ret, strerror(errno)); + return; + } + wait_for_child_and_detach(th); } static struct clnt_info * diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h index c6937c5..565bce3 100644 --- a/utils/gssd/gssd.h +++ b/utils/gssd/gssd.h @@ -36,6 +36,7 @@ #include <gssapi/gssapi.h> #include <event.h> #include <stdbool.h> +#include <pthread.h> #ifndef GSSD_PIPEFS_DIR #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" @@ -61,6 +62,10 @@ extern int root_uses_machine_creds; extern unsigned int context_timeout; extern unsigned int rpc_timeout; extern char *preferred_realm; +extern pthread_mutex_t ple_lock; +extern pthread_cond_t pcond; +extern pthread_mutex_t pmutex; +extern int thread_started; struct clnt_info { TAILQ_ENTRY(clnt_info) list; diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c index 1ef68d8..e2e95dc 100644 --- a/utils/gssd/gssd_proc.c +++ b/utils/gssd/gssd_proc.c @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname, if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 && service == NULL)) { - /* already running as uid 0 */ - if (uid == 0) - goto no_fork; - - pid = fork(); - switch(pid) { - case 0: - /* Child: fall through to rest of function */ - childpid = getpid(); - unsetenv("KRB5CCNAME"); - printerr(2, "CHILD forked pid %d \n", childpid); - break; - case -1: - /* fork() failed! */ - printerr(0, "WARNING: unable to fork() to handle" - "upcall: %s\n", strerror(errno)); - return; - default: - /* Parent: just wait on child to exit and return */ - do { - pid = wait(&err); - } while(pid == -1 && errno != -ECHILD); - - if (WIFSIGNALED(err)) - printerr(0, "WARNING: forked child was killed" - "with signal %d\n", WTERMSIG(err)); - return; - } -no_fork: - auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err, &err, &rpc_clnt); if (err) @@ -735,12 +705,28 @@ out_return_error: goto out; } +/* signal to the parent thread that we have read from the file descriptor. + * it should allow the parent to proceed to poll on the descriptor for + * the next upcall from the kernel. + */ +static void +signal_parent_event_consumed(void) +{ + pthread_mutex_lock(&pmutex); + thread_started = true; + pthread_cond_signal(&pcond); + pthread_mutex_unlock(&pmutex); +} + void handle_krb5_upcall(struct clnt_info *clp) { uid_t uid; + int status; - if (read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid)) { + status = read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid); + signal_parent_event_consumed(); + if (status) { printerr(0, "WARNING: failed reading uid from krb5 " "upcall pipe: %s\n", strerror(errno)); return; @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) char *enctypes = NULL; lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); + signal_parent_event_consumed(); if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { printerr(0, "WARNING: handle_gssd_upcall: " "failed reading request\n"); diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c index 8ef8184..3328696 100644 --- a/utils/gssd/krb5_util.c +++ b/utils/gssd/krb5_util.c @@ -128,6 +128,7 @@ /* Global list of principals/cache file names for machine credentials */ struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; #ifdef HAVE_SET_ALLOWABLE_ENCTYPES int limit_to_legacy_enctypes = 0; @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, krb5_principal princ) /* Need to serialize list if we ever become multi-threaded! */ + pthread_mutex_lock(&ple_lock); ple = find_ple_by_princ(context, princ); if (ple == NULL) { ple = new_ple(context, princ); } + pthread_mutex_unlock(&ple_lock); return ple; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia @ 2016-04-27 17:39 ` Steve Dickson 2016-04-28 13:13 ` Jeff Layton 1 sibling, 0 replies; 16+ messages in thread From: Steve Dickson @ 2016-04-27 17:39 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs On 04/27/2016 12:58 PM, Olga Kornievskaia wrote: > Currently, to persevere global data over multiple mounts, > the root process does not fork when handling an upcall. > Instead on not-forking create a pthread to handle the > upcall since global data can be shared among threads. > > Signed-off-by: Steve Dickson <steved@redhat.com> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > aclocal/libpthread.m4 | 13 +++++++++++++ > configure.ac | 3 +++ > utils/gssd/Makefile.am | 3 ++- > utils/gssd/gssd.c | 45 ++++++++++++++++++++++++++++++++++++++++----- > utils/gssd/gssd.h | 5 +++++ > utils/gssd/gssd_proc.c | 49 ++++++++++++++++++------------------------------- > utils/gssd/krb5_util.c | 3 +++ > 7 files changed, 84 insertions(+), 37 deletions(-) > create mode 100644 aclocal/libpthread.m4 > > diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 > new file mode 100644 > index 0000000..e87d2a0 > --- /dev/null > +++ b/aclocal/libpthread.m4 > @@ -0,0 +1,13 @@ > +dnl Checks for pthreads library and headers > +dnl > +AC_DEFUN([AC_LIBPTHREAD], [ > + > + dnl Check for library, but do not add -lpthreads to LIBS > + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=-lpthread], > + [AC_MSG_ERROR([libpthread not found.])]) > + AC_SUBST(LIBPTHREAD) > + > + AC_CHECK_HEADERS([pthread.h], , > + [AC_MSG_ERROR([libpthread headers not found.])]) > + > +])dnl > diff --git a/configure.ac b/configure.ac > index 25d2ba4..e0ecd61 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then > dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to have KRBLIBS set > AC_LIBRPCSECGSS > > + dnl Check for pthreads > + AC_LIBPTHREAD > + > dnl librpcsecgss already has a dependency on libgssapi, > dnl but we need to make sure we get the right version > if test "$enable_gss" = yes; then > diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am > index cb040b3..3f5f59a 100644 > --- a/utils/gssd/Makefile.am > +++ b/utils/gssd/Makefile.am > @@ -49,7 +49,8 @@ gssd_LDADD = \ > $(RPCSECGSS_LIBS) \ > $(KRBLIBS) \ > $(GSSAPI_LIBS) \ > - $(LIBTIRPC) > + $(LIBTIRPC) \ > + $(LIBPTHREAD) > > gssd_LDFLAGS = \ > $(KRBLDFLAGS) > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > index e7cb07f..b676341 100644 > --- a/utils/gssd/gssd.c > +++ b/utils/gssd/gssd.c > @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; > char *preferred_realm = NULL; > /* Avoid DNS reverse lookups on server names */ > static bool avoid_dns = true; > - > +int thread_started = false; > +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; > +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; > > TAILQ_HEAD(topdir_list_head, topdir) topdir_list; > > @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info *clp) > > static void gssd_scan(void); > > +static inline void wait_for_child_and_detach(pthread_t th) > +{ > + pthread_mutex_lock(&pmutex); > + while (!thread_started) > + pthread_cond_wait(&pcond, &pmutex); > + thread_started = false; > + pthread_mutex_unlock(&pmutex); > + pthread_detach(th); > +} > + > +/* For each upcall create a thread, detach from the main process so that > + * resources are released back into the system without the need for a join. > + * We need to wait for the child thread to start and consume the event from > + * the file descriptor. > + */ > static void > gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data) > { > struct clnt_info *clp = data; > - > - handle_gssd_upcall(clp); > + pthread_t th; > + int ret; > + > + ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall, > + (void *)clp); > + if (ret != 0) { > + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", > + ret, strerror(errno)); > + return; > + } > + wait_for_child_and_detach(th); > } > > static void > gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void *data) > { > struct clnt_info *clp = data; > - > - handle_krb5_upcall(clp); > + pthread_t th; > + int ret; > + > + ret = pthread_create(&th, NULL, (void *)handle_krb5_upcall, > + (void *)clp); > + if (ret != 0) { > + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", > + ret, strerror(errno)); > + return; > + } > + wait_for_child_and_detach(th); > } > > static struct clnt_info * > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > index c6937c5..565bce3 100644 > --- a/utils/gssd/gssd.h > +++ b/utils/gssd/gssd.h > @@ -36,6 +36,7 @@ > #include <gssapi/gssapi.h> > #include <event.h> > #include <stdbool.h> > +#include <pthread.h> > > #ifndef GSSD_PIPEFS_DIR > #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" > @@ -61,6 +62,10 @@ extern int root_uses_machine_creds; > extern unsigned int context_timeout; > extern unsigned int rpc_timeout; > extern char *preferred_realm; > +extern pthread_mutex_t ple_lock; > +extern pthread_cond_t pcond; > +extern pthread_mutex_t pmutex; > +extern int thread_started; > > struct clnt_info { > TAILQ_ENTRY(clnt_info) list; > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > index 1ef68d8..e2e95dc 100644 > --- a/utils/gssd/gssd_proc.c > +++ b/utils/gssd/gssd_proc.c > @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname, > if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 && > service == NULL)) { > > - /* already running as uid 0 */ > - if (uid == 0) > - goto no_fork; > - > - pid = fork(); > - switch(pid) { > - case 0: > - /* Child: fall through to rest of function */ > - childpid = getpid(); > - unsetenv("KRB5CCNAME"); > - printerr(2, "CHILD forked pid %d \n", childpid); > - break; > - case -1: > - /* fork() failed! */ > - printerr(0, "WARNING: unable to fork() to handle" > - "upcall: %s\n", strerror(errno)); > - return; > - default: > - /* Parent: just wait on child to exit and return */ > - do { > - pid = wait(&err); > - } while(pid == -1 && errno != -ECHILD); > - > - if (WIFSIGNALED(err)) > - printerr(0, "WARNING: forked child was killed" > - "with signal %d\n", WTERMSIG(err)); > - return; > - } > -no_fork: > - > auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err, > &err, &rpc_clnt); > if (err) > @@ -735,12 +705,28 @@ out_return_error: > goto out; > } > > +/* signal to the parent thread that we have read from the file descriptor. > + * it should allow the parent to proceed to poll on the descriptor for > + * the next upcall from the kernel. > + */ > +static void > +signal_parent_event_consumed(void) > +{ > + pthread_mutex_lock(&pmutex); > + thread_started = true; > + pthread_cond_signal(&pcond); > + pthread_mutex_unlock(&pmutex); > +} > + Small nit... this should be a static inline void as well... I'll fix it... no need to repost... steved. > void > handle_krb5_upcall(struct clnt_info *clp) > { > uid_t uid; > + int status; > > - if (read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid)) { > + status = read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid); > + signal_parent_event_consumed(); > + if (status) { > printerr(0, "WARNING: failed reading uid from krb5 " > "upcall pipe: %s\n", strerror(errno)); > return; > @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) > char *enctypes = NULL; > > lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); > + signal_parent_event_consumed(); > if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { > printerr(0, "WARNING: handle_gssd_upcall: " > "failed reading request\n"); > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > index 8ef8184..3328696 100644 > --- a/utils/gssd/krb5_util.c > +++ b/utils/gssd/krb5_util.c > @@ -128,6 +128,7 @@ > > /* Global list of principals/cache file names for machine credentials */ > struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; > +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; > > #ifdef HAVE_SET_ALLOWABLE_ENCTYPES > int limit_to_legacy_enctypes = 0; > @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, krb5_principal princ) > > /* Need to serialize list if we ever become multi-threaded! */ > > + pthread_mutex_lock(&ple_lock); > ple = find_ple_by_princ(context, princ); > if (ple == NULL) { > ple = new_ple(context, princ); > } > + pthread_mutex_unlock(&ple_lock); > > return ple; > } > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia 2016-04-27 17:39 ` Steve Dickson @ 2016-04-28 13:13 ` Jeff Layton 2016-04-28 14:07 ` Kornievskaia, Olga 1 sibling, 1 reply; 16+ messages in thread From: Jeff Layton @ 2016-04-28 13:13 UTC (permalink / raw) To: Olga Kornievskaia, steved; +Cc: linux-nfs On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: > Currently, to persevere global data over multiple mounts, > the root process does not fork when handling an upcall. > Instead on not-forking create a pthread to handle the > upcall since global data can be shared among threads. > > Signed-off-by: Steve Dickson <steved@redhat.com> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > aclocal/libpthread.m4 | 13 +++++++++++++ > configure.ac | 3 +++ > utils/gssd/Makefile.am | 3 ++- > utils/gssd/gssd.c | 45 ++++++++++++++++++++++++++++++++++++++++----- > utils/gssd/gssd.h | 5 +++++ > utils/gssd/gssd_proc.c | 49 ++++++++++++++++++------------------------------- > utils/gssd/krb5_util.c | 3 +++ > 7 files changed, 84 insertions(+), 37 deletions(-) > create mode 100644 aclocal/libpthread.m4 > > diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 > new file mode 100644 > index 0000000..e87d2a0 > --- /dev/null > +++ b/aclocal/libpthread.m4 > @@ -0,0 +1,13 @@ > +dnl Checks for pthreads library and headers > +dnl > +AC_DEFUN([AC_LIBPTHREAD], [ > + > + dnl Check for library, but do not add -lpthreads to LIBS > + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=-lpthread], > + [AC_MSG_ERROR([libpthread not found.])]) > + AC_SUBST(LIBPTHREAD) > + > + AC_CHECK_HEADERS([pthread.h], , > + [AC_MSG_ERROR([libpthread headers not found.])]) > + > +])dnl > diff --git a/configure.ac b/configure.ac > index 25d2ba4..e0ecd61 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then > dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to have KRBLIBS set > AC_LIBRPCSECGSS > > + dnl Check for pthreads > + AC_LIBPTHREAD > + > dnl librpcsecgss already has a dependency on libgssapi, > dnl but we need to make sure we get the right version > if test "$enable_gss" = yes; then > diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am > index cb040b3..3f5f59a 100644 > --- a/utils/gssd/Makefile.am > +++ b/utils/gssd/Makefile.am > @@ -49,7 +49,8 @@ gssd_LDADD = \ > $(RPCSECGSS_LIBS) \ > $(KRBLIBS) \ > $(GSSAPI_LIBS) \ > - $(LIBTIRPC) > + $(LIBTIRPC) \ > + $(LIBPTHREAD) > > gssd_LDFLAGS = \ > $(KRBLDFLAGS) > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > index e7cb07f..b676341 100644 > --- a/utils/gssd/gssd.c > +++ b/utils/gssd/gssd.c > @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; > char *preferred_realm = NULL; > /* Avoid DNS reverse lookups on server names */ > static bool avoid_dns = true; > - > +int thread_started = false; > +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; > +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; > > TAILQ_HEAD(topdir_list_head, topdir) topdir_list; > > @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info *clp) > > static void gssd_scan(void); > > +static inline void wait_for_child_and_detach(pthread_t th) > +{ > + pthread_mutex_lock(&pmutex); > + while (!thread_started) > + pthread_cond_wait(&pcond, &pmutex); > + thread_started = false; > + pthread_mutex_unlock(&pmutex); > + pthread_detach(th); > +} > + > +/* For each upcall create a thread, detach from the main process so that > + * resources are released back into the system without the need for a join. > + * We need to wait for the child thread to start and consume the event from > + * the file descriptor. > + */ > static void > gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data) > { > struct clnt_info *clp = data; > - > - handle_gssd_upcall(clp); > + pthread_t th; > + int ret; > + > + ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall, > + (void *)clp); > + if (ret != 0) { > + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", > + ret, strerror(errno)); > + return; > + } > + wait_for_child_and_detach(th); > } > > static void > gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void *data) > { > struct clnt_info *clp = data; > - > - handle_krb5_upcall(clp); > + pthread_t th; > + int ret; > + > + ret = pthread_create(&th, NULL, (void *)handle_krb5_upcall, > + (void *)clp); > + if (ret != 0) { > + printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", > + ret, strerror(errno)); > + return; > + } > + wait_for_child_and_detach(th); > } > > static struct clnt_info * > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > index c6937c5..565bce3 100644 > --- a/utils/gssd/gssd.h > +++ b/utils/gssd/gssd.h > @@ -36,6 +36,7 @@ > #include > #include > #include > +#include > > #ifndef GSSD_PIPEFS_DIR > #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" > @@ -61,6 +62,10 @@ extern int root_uses_machine_creds; > extern unsigned int context_timeout; > extern unsigned int rpc_timeout; > extern char *preferred_realm; > +extern pthread_mutex_t ple_lock; > +extern pthread_cond_t pcond; > +extern pthread_mutex_t pmutex; > +extern int thread_started; > > struct clnt_info { > TAILQ_ENTRY(clnt_info) list; > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > index 1ef68d8..e2e95dc 100644 > --- a/utils/gssd/gssd_proc.c > +++ b/utils/gssd/gssd_proc.c > @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname, > if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 && > service == NULL)) { > > - /* already running as uid 0 */ > - if (uid == 0) > - goto no_fork; > - > - pid = fork(); > - switch(pid) { > - case 0: > - /* Child: fall through to rest of function */ > - childpid = getpid(); > - unsetenv("KRB5CCNAME"); > - printerr(2, "CHILD forked pid %d \n", childpid); > - break; > - case -1: > - /* fork() failed! */ > - printerr(0, "WARNING: unable to fork() to handle" > - "upcall: %s\n", strerror(errno)); > - return; > - default: > - /* Parent: just wait on child to exit and return */ > - do { > - pid = wait(&err); > - } while(pid == -1 && errno != -ECHILD); > - > - if (WIFSIGNALED(err)) > - printerr(0, "WARNING: forked child was killed" > - "with signal %d\n", WTERMSIG(err)); > - return; > - } > -no_fork: > - > auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err, > &err, &rpc_clnt); > if (err) > @@ -735,12 +705,28 @@ out_return_error: > goto out; > } > > +/* signal to the parent thread that we have read from the file descriptor. > + * it should allow the parent to proceed to poll on the descriptor for > + * the next upcall from the kernel. > + */ > +static void > +signal_parent_event_consumed(void) > +{ > + pthread_mutex_lock(&pmutex); > + thread_started = true; > + pthread_cond_signal(&pcond); > + pthread_mutex_unlock(&pmutex); > +} > + > void > handle_krb5_upcall(struct clnt_info *clp) > { > uid_t uid; > + int status; > > - if (read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid)) { > + status = read(clp->krb5_fd, &uid, sizeof(uid)) < (ssize_t)sizeof(uid); > + signal_parent_event_consumed(); > + if (status) { > printerr(0, "WARNING: failed reading uid from krb5 " > "upcall pipe: %s\n", strerror(errno)); > return; > @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) > char *enctypes = NULL; > > lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); > + signal_parent_event_consumed(); > if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { > printerr(0, "WARNING: handle_gssd_upcall: " > "failed reading request\n"); > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > index 8ef8184..3328696 100644 > --- a/utils/gssd/krb5_util.c > +++ b/utils/gssd/krb5_util.c > @@ -128,6 +128,7 @@ > > /* Global list of principals/cache file names for machine credentials */ > struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; > +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; > > #ifdef HAVE_SET_ALLOWABLE_ENCTYPES > int limit_to_legacy_enctypes = 0; > @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, krb5_principal princ) > > /* Need to serialize list if we ever become multi-threaded! */ > > + pthread_mutex_lock(&ple_lock); > ple = find_ple_by_princ(context, princ); > if (ple == NULL) { > ple = new_ple(context, princ); > } > + pthread_mutex_unlock(&ple_lock); > > return ple; > } Looks good. One thing that might be neat as a follow up is to make the thread_started variable a per-clnt thing. I don't think there's any reason to serialize I/O to different fds there. Reviewed-by: Jeff Layton <jlayton@poochiereds.net> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-28 13:13 ` Jeff Layton @ 2016-04-28 14:07 ` Kornievskaia, Olga 2016-04-30 12:29 ` Jeff Layton 0 siblings, 1 reply; 16+ messages in thread From: Kornievskaia, Olga @ 2016-04-28 14:07 UTC (permalink / raw) To: Jeff Layton; +Cc: steved, linux-nfs DQo+IE9uIEFwciAyOCwgMjAxNiwgYXQgOToxMyBBTSwgSmVmZiBMYXl0b24gPGpsYXl0b25AcG9v Y2hpZXJlZHMubmV0PiB3cm90ZToNCj4gDQo+IE9uIFdlZCwgMjAxNi0wNC0yNyBhdCAxMjo1OCAt MDQwMCwgT2xnYSBLb3JuaWV2c2thaWEgd3JvdGU6DQo+PiBDdXJyZW50bHksIHRvIHBlcnNldmVy ZSBnbG9iYWwgZGF0YSBvdmVyIG11bHRpcGxlIG1vdW50cywNCj4+IHRoZSByb290IHByb2Nlc3Mg ZG9lcyBub3QgZm9yayB3aGVuIGhhbmRsaW5nIGFuIHVwY2FsbC4NCj4+IEluc3RlYWQgb24gbm90 LWZvcmtpbmcgY3JlYXRlIGEgcHRocmVhZCB0byBoYW5kbGUgdGhlDQo+PiB1cGNhbGwgc2luY2Ug Z2xvYmFsIGRhdGEgY2FuIGJlIHNoYXJlZCBhbW9uZyB0aHJlYWRzLg0KPj4gDQo+PiBTaWduZWQt b2ZmLWJ5OiBTdGV2ZSBEaWNrc29uIDxzdGV2ZWRAcmVkaGF0LmNvbT4NCj4+IFNpZ25lZC1vZmYt Ynk6IE9sZ2EgS29ybmlldnNrYWlhIDxrb2xnYUBuZXRhcHAuY29tPg0KPj4gLS0tDQo+PiAgYWNs b2NhbC9saWJwdGhyZWFkLm00ICB8IDEzICsrKysrKysrKysrKysNCj4+ICBjb25maWd1cmUuYWMg ICAgICAgICAgIHwgIDMgKysrDQo+PiAgdXRpbHMvZ3NzZC9NYWtlZmlsZS5hbSB8ICAzICsrLQ0K Pj4gIHV0aWxzL2dzc2QvZ3NzZC5jICAgICAgfCA0NSArKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrLS0tLS0NCj4+ICB1dGlscy9nc3NkL2dzc2QuaCAgICAgIHwgIDUgKysr KysNCj4+ICB1dGlscy9nc3NkL2dzc2RfcHJvYy5jIHwgNDkgKysrKysrKysrKysrKysrKysrLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPj4gIHV0aWxzL2dzc2Qva3JiNV91dGlsLmMg fCAgMyArKysNCj4+ICA3IGZpbGVzIGNoYW5nZWQsIDg0IGluc2VydGlvbnMoKyksIDM3IGRlbGV0 aW9ucygtKQ0KPj4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBhY2xvY2FsL2xpYnB0aHJlYWQubTQNCj4+ IA0KPj4gZGlmZiAtLWdpdCBhL2FjbG9jYWwvbGlicHRocmVhZC5tNCBiL2FjbG9jYWwvbGlicHRo cmVhZC5tNA0KPj4gbmV3IGZpbGUgbW9kZSAxMDA2NDQNCj4+IGluZGV4IDAwMDAwMDAuLmU4N2Qy YTANCj4+IC0tLSAvZGV2L251bGwNCj4+ICsrKyBiL2FjbG9jYWwvbGlicHRocmVhZC5tNA0KPj4g QEAgLTAsMCArMSwxMyBAQA0KPj4gK2RubCBDaGVja3MgZm9yIHB0aHJlYWRzIGxpYnJhcnkgYW5k IGhlYWRlcnMNCj4+ICtkbmwNCj4+ICtBQ19ERUZVTihbQUNfTElCUFRIUkVBRF0sIFsNCj4+ICsN Cj4+ICsgICAgZG5sIENoZWNrIGZvciBsaWJyYXJ5LCBidXQgZG8gbm90IGFkZCAtbHB0aHJlYWRz IHRvIExJQlMNCj4+ICsgICAgQUNfQ0hFQ0tfTElCKFtwdGhyZWFkXSwgW3B0aHJlYWRfY3JlYXRl XSwgW0xJQlBUSFJFQUQ9LWxwdGhyZWFkXSwNCj4+ICsgICAgICAgICAgICAgICAgIFtBQ19NU0df RVJST1IoW2xpYnB0aHJlYWQgbm90IGZvdW5kLl0pXSkNCj4+ICsgIEFDX1NVQlNUKExJQlBUSFJF QUQpDQo+PiArDQo+PiArICBBQ19DSEVDS19IRUFERVJTKFtwdGhyZWFkLmhdLCAsDQo+PiArICAg ICAgICAgICAgICAgICAgIFtBQ19NU0dfRVJST1IoW2xpYnB0aHJlYWQgaGVhZGVycyBub3QgZm91 bmQuXSldKQ0KPj4gKw0KPj4gK10pZG5sDQo+PiBkaWZmIC0tZ2l0IGEvY29uZmlndXJlLmFjIGIv Y29uZmlndXJlLmFjDQo+PiBpbmRleCAyNWQyYmE0Li5lMGVjZDYxIDEwMDY0NA0KPj4gLS0tIGEv Y29uZmlndXJlLmFjDQo+PiArKysgYi9jb25maWd1cmUuYWMNCj4+IEBAIC0zODUsNiArMzg1LDkg QEAgaWYgdGVzdCAiJGVuYWJsZV9nc3MiID0geWVzOyB0aGVuDQo+PiAgICBkbmwgSW52b2tlZCBh ZnRlciBBQ19LRVJCRVJPU19WNTsgQUNfTElCUlBDU0VDR1NTIG5lZWRzIHRvIGhhdmUgS1JCTElC UyBzZXQNCj4+ICAgIEFDX0xJQlJQQ1NFQ0dTUw0KPj4gIA0KPj4gKyAgZG5sIENoZWNrIGZvciBw dGhyZWFkcw0KPj4gKyAgQUNfTElCUFRIUkVBRA0KPj4gKw0KPj4gICAgZG5sIGxpYnJwY3NlY2dz cyBhbHJlYWR5IGhhcyBhIGRlcGVuZGVuY3kgb24gbGliZ3NzYXBpLA0KPj4gICAgZG5sIGJ1dCB3 ZSBuZWVkIHRvIG1ha2Ugc3VyZSB3ZSBnZXQgdGhlIHJpZ2h0IHZlcnNpb24NCj4+ICAgIGlmIHRl c3QgIiRlbmFibGVfZ3NzIiA9IHllczsgdGhlbg0KPj4gZGlmZiAtLWdpdCBhL3V0aWxzL2dzc2Qv TWFrZWZpbGUuYW0gYi91dGlscy9nc3NkL01ha2VmaWxlLmFtDQo+PiBpbmRleCBjYjA0MGIzLi4z ZjVmNTlhIDEwMDY0NA0KPj4gLS0tIGEvdXRpbHMvZ3NzZC9NYWtlZmlsZS5hbQ0KPj4gKysrIGIv dXRpbHMvZ3NzZC9NYWtlZmlsZS5hbQ0KPj4gQEAgLTQ5LDcgKzQ5LDggQEAgZ3NzZF9MREFERCA9 IFwNCj4+ICAJJChSUENTRUNHU1NfTElCUykgXA0KPj4gIAkkKEtSQkxJQlMpIFwNCj4+ICAJJChH U1NBUElfTElCUykgXA0KPj4gLQkkKExJQlRJUlBDKQ0KPj4gKwkkKExJQlRJUlBDKSBcDQo+PiAr CSQoTElCUFRIUkVBRCkNCj4+ICANCj4+ICBnc3NkX0xERkxBR1MgPSBcDQo+PiAgCSQoS1JCTERG TEFHUykNCj4+IGRpZmYgLS1naXQgYS91dGlscy9nc3NkL2dzc2QuYyBiL3V0aWxzL2dzc2QvZ3Nz ZC5jDQo+PiBpbmRleCBlN2NiMDdmLi5iNjc2MzQxIDEwMDY0NA0KPj4gLS0tIGEvdXRpbHMvZ3Nz ZC9nc3NkLmMNCj4+ICsrKyBiL3V0aWxzL2dzc2QvZ3NzZC5jDQo+PiBAQCAtODcsNyArODcsOSBA QCB1bnNpZ25lZCBpbnQgIHJwY190aW1lb3V0ID0gNTsNCj4+ICBjaGFyICpwcmVmZXJyZWRfcmVh bG0gPSBOVUxMOw0KPj4gIC8qIEF2b2lkIEROUyByZXZlcnNlIGxvb2t1cHMgb24gc2VydmVyIG5h bWVzICovDQo+PiAgc3RhdGljIGJvb2wgYXZvaWRfZG5zID0gdHJ1ZTsNCj4+IC0NCj4+ICtpbnQg dGhyZWFkX3N0YXJ0ZWQgPSBmYWxzZTsNCj4+ICtwdGhyZWFkX211dGV4X3QgcG11dGV4ID0gUFRI UkVBRF9NVVRFWF9JTklUSUFMSVpFUjsNCj4+ICtwdGhyZWFkX2NvbmRfdCBwY29uZCA9IFBUSFJF QURfQ09ORF9JTklUSUFMSVpFUjsNCj4+ICANCj4+ICBUQUlMUV9IRUFEKHRvcGRpcl9saXN0X2hl YWQsIHRvcGRpcikgdG9wZGlyX2xpc3Q7DQo+PiAgDQo+PiBAQCAtMzYxLDIwICszNjMsNTMgQEAg Z3NzZF9kZXN0cm95X2NsaWVudChzdHJ1Y3QgY2xudF9pbmZvICpjbHApDQo+PiAgDQo+PiAgc3Rh dGljIHZvaWQgZ3NzZF9zY2FuKHZvaWQpOw0KPj4gIA0KPj4gK3N0YXRpYyBpbmxpbmUgdm9pZCB3 YWl0X2Zvcl9jaGlsZF9hbmRfZGV0YWNoKHB0aHJlYWRfdCB0aCkNCj4+ICt7DQo+PiArCXB0aHJl YWRfbXV0ZXhfbG9jaygmcG11dGV4KTsNCj4+ICsJd2hpbGUgKCF0aHJlYWRfc3RhcnRlZCkNCj4+ ICsJCXB0aHJlYWRfY29uZF93YWl0KCZwY29uZCwgJnBtdXRleCk7DQo+PiArCXRocmVhZF9zdGFy dGVkID0gZmFsc2U7DQo+PiArCXB0aHJlYWRfbXV0ZXhfdW5sb2NrKCZwbXV0ZXgpOw0KPj4gKwlw dGhyZWFkX2RldGFjaCh0aCk7DQo+PiArfQ0KPj4gKw0KPj4gKy8qIEZvciBlYWNoIHVwY2FsbCBj cmVhdGUgYSB0aHJlYWQsIGRldGFjaCBmcm9tIHRoZSBtYWluIHByb2Nlc3Mgc28gdGhhdA0KPj4g KyAqIHJlc291cmNlcyBhcmUgcmVsZWFzZWQgYmFjayBpbnRvIHRoZSBzeXN0ZW0gd2l0aG91dCB0 aGUgbmVlZCBmb3IgYSBqb2luLg0KPj4gKyAqIFdlIG5lZWQgdG8gd2FpdCBmb3IgdGhlIGNoaWxk IHRocmVhZCB0byBzdGFydCBhbmQgY29uc3VtZSB0aGUgZXZlbnQgZnJvbQ0KPj4gKyAqIHRoZSBm aWxlIGRlc2NyaXB0b3IuDQo+PiArICovDQo+PiAgc3RhdGljIHZvaWQNCj4+ICBnc3NkX2NsbnRf Z3NzZF9jYihpbnQgVU5VU0VEKGZkKSwgc2hvcnQgVU5VU0VEKHdoaWNoKSwgdm9pZCAqZGF0YSkN Cj4+ICB7DQo+PiAgCXN0cnVjdCBjbG50X2luZm8gKmNscCA9IGRhdGE7DQo+PiAtDQo+PiAtCWhh bmRsZV9nc3NkX3VwY2FsbChjbHApOw0KPj4gKwlwdGhyZWFkX3QgdGg7DQo+PiArCWludCByZXQ7 DQo+PiArDQo+PiArCXJldCA9IHB0aHJlYWRfY3JlYXRlKCZ0aCwgTlVMTCwgKHZvaWQgKiloYW5k bGVfZ3NzZF91cGNhbGwsDQo+PiArCQkJCSh2b2lkICopY2xwKTsNCj4+ICsJaWYgKHJldCAhPSAw KSB7DQo+PiArCQlwcmludGVycigwLCAiRVJST1I6IHB0aHJlYWRfY3JlYXRlIGZhaWxlZDogcmV0 ICVkOiAlc1xuIiwNCj4+ICsJCQkgcmV0LCBzdHJlcnJvcihlcnJubykpOw0KPj4gKwkJcmV0dXJu Ow0KPj4gKwl9DQo+PiArCXdhaXRfZm9yX2NoaWxkX2FuZF9kZXRhY2godGgpOw0KPj4gIH0NCj4+ ICANCj4+ICBzdGF0aWMgdm9pZA0KPj4gIGdzc2RfY2xudF9rcmI1X2NiKGludCBVTlVTRUQoZmQp LCBzaG9ydCBVTlVTRUQod2hpY2gpLCB2b2lkICpkYXRhKQ0KPj4gIHsNCj4+ICAJc3RydWN0IGNs bnRfaW5mbyAqY2xwID0gZGF0YTsNCj4+IC0NCj4+IC0JaGFuZGxlX2tyYjVfdXBjYWxsKGNscCk7 DQo+PiArCXB0aHJlYWRfdCB0aDsNCj4+ICsJaW50IHJldDsNCj4+ICsNCj4+ICsJcmV0ID0gcHRo cmVhZF9jcmVhdGUoJnRoLCBOVUxMLCAodm9pZCAqKWhhbmRsZV9rcmI1X3VwY2FsbCwNCj4+ICsJ CQkJKHZvaWQgKiljbHApOw0KPj4gKwlpZiAocmV0ICE9IDApIHsNCj4+ICsJCXByaW50ZXJyKDAs ICJFUlJPUjogcHRocmVhZF9jcmVhdGUgZmFpbGVkOiByZXQgJWQ6ICVzXG4iLA0KPj4gKwkJCSBy ZXQsIHN0cmVycm9yKGVycm5vKSk7DQo+PiArCQlyZXR1cm47DQo+PiArCX0NCj4+ICsJd2FpdF9m b3JfY2hpbGRfYW5kX2RldGFjaCh0aCk7DQo+PiAgfQ0KPj4gIA0KPj4gIHN0YXRpYyBzdHJ1Y3Qg Y2xudF9pbmZvICoNCj4+IGRpZmYgLS1naXQgYS91dGlscy9nc3NkL2dzc2QuaCBiL3V0aWxzL2dz c2QvZ3NzZC5oDQo+PiBpbmRleCBjNjkzN2M1Li41NjViY2UzIDEwMDY0NA0KPj4gLS0tIGEvdXRp bHMvZ3NzZC9nc3NkLmgNCj4+ICsrKyBiL3V0aWxzL2dzc2QvZ3NzZC5oDQo+PiBAQCAtMzYsNiAr MzYsNyBAQA0KPj4gICNpbmNsdWRlIA0KPj4gICNpbmNsdWRlIA0KPj4gICNpbmNsdWRlIA0KPj4g KyNpbmNsdWRlIA0KPj4gIA0KPj4gICNpZm5kZWYgR1NTRF9QSVBFRlNfRElSDQo+PiAgI2RlZmlu ZSBHU1NEX1BJUEVGU19ESVIJCSIvdmFyL2xpYi9uZnMvcnBjX3BpcGVmcyINCj4+IEBAIC02MSw2 ICs2MiwxMCBAQCBleHRlcm4gaW50CQkJcm9vdF91c2VzX21hY2hpbmVfY3JlZHM7DQo+PiAgZXh0 ZXJuIHVuc2lnbmVkIGludCAJCWNvbnRleHRfdGltZW91dDsNCj4+ICBleHRlcm4gdW5zaWduZWQg aW50IHJwY190aW1lb3V0Ow0KPj4gIGV4dGVybiBjaGFyCQkJKnByZWZlcnJlZF9yZWFsbTsNCj4+ ICtleHRlcm4gcHRocmVhZF9tdXRleF90IHBsZV9sb2NrOw0KPj4gK2V4dGVybiBwdGhyZWFkX2Nv bmRfdCBwY29uZDsNCj4+ICtleHRlcm4gcHRocmVhZF9tdXRleF90IHBtdXRleDsNCj4+ICtleHRl cm4gaW50IHRocmVhZF9zdGFydGVkOw0KPj4gIA0KPj4gIHN0cnVjdCBjbG50X2luZm8gew0KPj4g IAlUQUlMUV9FTlRSWShjbG50X2luZm8pCWxpc3Q7DQo+PiBkaWZmIC0tZ2l0IGEvdXRpbHMvZ3Nz ZC9nc3NkX3Byb2MuYyBiL3V0aWxzL2dzc2QvZ3NzZF9wcm9jLmMNCj4+IGluZGV4IDFlZjY4ZDgu LmUyZTk1ZGMgMTAwNjQ0DQo+PiAtLS0gYS91dGlscy9nc3NkL2dzc2RfcHJvYy5jDQo+PiArKysg Yi91dGlscy9nc3NkL2dzc2RfcHJvYy5jDQo+PiBAQCAtNjI5LDM2ICs2MjksNiBAQCBwcm9jZXNz X2tyYjVfdXBjYWxsKHN0cnVjdCBjbG50X2luZm8gKmNscCwgdWlkX3QgdWlkLCBpbnQgZmQsIGNo YXIgKnRndG5hbWUsDQo+PiAgCWlmICh1aWQgIT0gMCB8fCAodWlkID09IDAgJiYgcm9vdF91c2Vz X21hY2hpbmVfY3JlZHMgPT0gMCAmJg0KPj4gIAkJCQlzZXJ2aWNlID09IE5VTEwpKSB7DQo+PiAg DQo+PiAtCQkvKiBhbHJlYWR5IHJ1bm5pbmcgYXMgdWlkIDAgKi8NCj4+IC0JCWlmICh1aWQgPT0g MCkNCj4+IC0JCQlnb3RvIG5vX2Zvcms7DQo+PiAtDQo+PiAtCQlwaWQgPSBmb3JrKCk7DQo+PiAt CQlzd2l0Y2gocGlkKSB7DQo+PiAtCQljYXNlIDA6DQo+PiAtCQkJLyogQ2hpbGQ6IGZhbGwgdGhy b3VnaCB0byByZXN0IG9mIGZ1bmN0aW9uICovDQo+PiAtCQkJY2hpbGRwaWQgPSBnZXRwaWQoKTsN Cj4+IC0JCQl1bnNldGVudigiS1JCNUNDTkFNRSIpOw0KPj4gLQkJCXByaW50ZXJyKDIsICJDSElM RCBmb3JrZWQgcGlkICVkIFxuIiwgY2hpbGRwaWQpOw0KPj4gLQkJCWJyZWFrOw0KPj4gLQkJY2Fz ZSAtMToNCj4+IC0JCQkvKiBmb3JrKCkgZmFpbGVkISAqLw0KPj4gLQkJCXByaW50ZXJyKDAsICJX QVJOSU5HOiB1bmFibGUgdG8gZm9yaygpIHRvIGhhbmRsZSINCj4+IC0JCQkJInVwY2FsbDogJXNc biIsIHN0cmVycm9yKGVycm5vKSk7DQo+PiAtCQkJcmV0dXJuOw0KPj4gLQkJZGVmYXVsdDoNCj4+ IC0JCQkvKiBQYXJlbnQ6IGp1c3Qgd2FpdCBvbiBjaGlsZCB0byBleGl0IGFuZCByZXR1cm4gKi8N Cj4+IC0JCQlkbyB7DQo+PiAtCQkJCXBpZCA9IHdhaXQoJmVycik7DQo+PiAtCQkJfSB3aGlsZShw aWQgPT0gLTEgJiYgZXJybm8gIT0gLUVDSElMRCk7DQo+PiAtDQo+PiAtCQkJaWYgKFdJRlNJR05B TEVEKGVycikpDQo+PiAtCQkJCXByaW50ZXJyKDAsICJXQVJOSU5HOiBmb3JrZWQgY2hpbGQgd2Fz IGtpbGxlZCINCj4+IC0JCQkJCSAid2l0aCBzaWduYWwgJWRcbiIsIFdURVJNU0lHKGVycikpOw0K Pj4gLQkJCXJldHVybjsNCj4+IC0JCX0NCj4+IC1ub19mb3JrOg0KPj4gLQ0KPj4gIAkJYXV0aCA9 IGtyYjVfbm90X21hY2hpbmVfY3JlZHMoY2xwLCB1aWQsIHRndG5hbWUsICZkb3duY2FsbF9lcnIs DQo+PiAgCQkJCQkJJmVyciwgJnJwY19jbG50KTsNCj4+ICAJCWlmIChlcnIpDQo+PiBAQCAtNzM1 LDEyICs3MDUsMjggQEAgb3V0X3JldHVybl9lcnJvcjoNCj4+ICAJZ290byBvdXQ7DQo+PiAgfQ0K Pj4gIA0KPj4gKy8qIHNpZ25hbCB0byB0aGUgcGFyZW50IHRocmVhZCB0aGF0IHdlIGhhdmUgcmVh ZCBmcm9tIHRoZSBmaWxlIGRlc2NyaXB0b3IuDQo+PiArICogaXQgc2hvdWxkIGFsbG93IHRoZSBw YXJlbnQgdG8gcHJvY2VlZCB0byBwb2xsIG9uIHRoZSBkZXNjcmlwdG9yIGZvcg0KPj4gKyAqIHRo ZSBuZXh0IHVwY2FsbCBmcm9tIHRoZSBrZXJuZWwuDQo+PiArICovDQo+PiArc3RhdGljIHZvaWQN Cj4+ICtzaWduYWxfcGFyZW50X2V2ZW50X2NvbnN1bWVkKHZvaWQpDQo+PiArew0KPj4gKwlwdGhy ZWFkX211dGV4X2xvY2soJnBtdXRleCk7DQo+PiArCXRocmVhZF9zdGFydGVkID0gdHJ1ZTsNCj4+ ICsJcHRocmVhZF9jb25kX3NpZ25hbCgmcGNvbmQpOw0KPj4gKwlwdGhyZWFkX211dGV4X3VubG9j aygmcG11dGV4KTsNCj4+ICt9DQo+PiArDQo+PiAgdm9pZA0KPj4gIGhhbmRsZV9rcmI1X3VwY2Fs bChzdHJ1Y3QgY2xudF9pbmZvICpjbHApDQo+PiAgew0KPj4gIAl1aWRfdAkJCXVpZDsNCj4+ICsJ aW50IAkJCXN0YXR1czsNCj4+ICANCj4+IC0JaWYgKHJlYWQoY2xwLT5rcmI1X2ZkLCAmdWlkLCBz aXplb2YodWlkKSkgPCAoc3NpemVfdClzaXplb2YodWlkKSkgew0KPj4gKwlzdGF0dXMgPSByZWFk KGNscC0+a3JiNV9mZCwgJnVpZCwgc2l6ZW9mKHVpZCkpIDwgKHNzaXplX3Qpc2l6ZW9mKHVpZCk7 DQo+PiArCXNpZ25hbF9wYXJlbnRfZXZlbnRfY29uc3VtZWQoKTsNCj4+ICsJaWYgKHN0YXR1cykg ew0KPj4gIAkJcHJpbnRlcnIoMCwgIldBUk5JTkc6IGZhaWxlZCByZWFkaW5nIHVpZCBmcm9tIGty YjUgIg0KPj4gIAkJCSAgICAidXBjYWxsIHBpcGU6ICVzXG4iLCBzdHJlcnJvcihlcnJubykpOw0K Pj4gIAkJcmV0dXJuOw0KPj4gQEAgLTc2NSw2ICs3NTEsNyBAQCBoYW5kbGVfZ3NzZF91cGNhbGwo c3RydWN0IGNsbnRfaW5mbyAqY2xwKQ0KPj4gIAljaGFyCQkJKmVuY3R5cGVzID0gTlVMTDsNCj4+ ICANCj4+ICAJbGJ1ZmxlbiA9IHJlYWQoY2xwLT5nc3NkX2ZkLCBsYnVmLCBzaXplb2YobGJ1Zikp Ow0KPj4gKwlzaWduYWxfcGFyZW50X2V2ZW50X2NvbnN1bWVkKCk7DQo+PiAgCWlmIChsYnVmbGVu IDw9IDAgfHwgbGJ1ZltsYnVmbGVuLTFdICE9ICdcbicpIHsNCj4+ICAJCXByaW50ZXJyKDAsICJX QVJOSU5HOiBoYW5kbGVfZ3NzZF91cGNhbGw6ICINCj4+ICAJCQkgICAgImZhaWxlZCByZWFkaW5n IHJlcXVlc3RcbiIpOw0KPj4gZGlmZiAtLWdpdCBhL3V0aWxzL2dzc2Qva3JiNV91dGlsLmMgYi91 dGlscy9nc3NkL2tyYjVfdXRpbC5jDQo+PiBpbmRleCA4ZWY4MTg0Li4zMzI4Njk2IDEwMDY0NA0K Pj4gLS0tIGEvdXRpbHMvZ3NzZC9rcmI1X3V0aWwuYw0KPj4gKysrIGIvdXRpbHMvZ3NzZC9rcmI1 X3V0aWwuYw0KPj4gQEAgLTEyOCw2ICsxMjgsNyBAQA0KPj4gIA0KPj4gIC8qIEdsb2JhbCBsaXN0 IG9mIHByaW5jaXBhbHMvY2FjaGUgZmlsZSBuYW1lcyBmb3IgbWFjaGluZSBjcmVkZW50aWFscyAq Lw0KPj4gIHN0cnVjdCBnc3NkX2s1X2t0X3ByaW5jICpnc3NkX2s1X2t0X3ByaW5jX2xpc3QgPSBO VUxMOw0KPj4gK3B0aHJlYWRfbXV0ZXhfdCBwbGVfbG9jayA9IFBUSFJFQURfTVVURVhfSU5JVElB TElaRVI7DQo+PiAgDQo+PiAgI2lmZGVmIEhBVkVfU0VUX0FMTE9XQUJMRV9FTkNUWVBFUw0KPj4g IGludCBsaW1pdF90b19sZWdhY3lfZW5jdHlwZXMgPSAwOw0KPj4gQEAgLTU4NiwxMCArNTg3LDEy IEBAIGdldF9wbGVfYnlfcHJpbmMoa3JiNV9jb250ZXh0IGNvbnRleHQsIGtyYjVfcHJpbmNpcGFs IHByaW5jKQ0KPj4gIA0KPj4gIAkvKiBOZWVkIHRvIHNlcmlhbGl6ZSBsaXN0IGlmIHdlIGV2ZXIg YmVjb21lIG11bHRpLXRocmVhZGVkISAqLw0KPj4gIA0KPj4gKwlwdGhyZWFkX211dGV4X2xvY2so JnBsZV9sb2NrKTsNCj4+ICAJcGxlID0gZmluZF9wbGVfYnlfcHJpbmMoY29udGV4dCwgcHJpbmMp Ow0KPj4gIAlpZiAocGxlID09IE5VTEwpIHsNCj4+ICAJCXBsZSA9IG5ld19wbGUoY29udGV4dCwg cHJpbmMpOw0KPj4gIAl9DQo+PiArCXB0aHJlYWRfbXV0ZXhfdW5sb2NrKCZwbGVfbG9jayk7DQo+ PiAgDQo+PiAgCXJldHVybiBwbGU7DQo+PiAgfQ0KPiANCj4gTG9va3MgZ29vZC4gT25lIHRoaW5n IHRoYXQgbWlnaHQgYmUgbmVhdCBhcyBhIGZvbGxvdyB1cCBpcyB0byBtYWtlIHRoZQ0KPiB0aHJl YWRfc3RhcnRlZCB2YXJpYWJsZSBhIHBlci1jbG50IHRoaW5nLiBJIGRvbid0IHRoaW5rIHRoZXJl J3MgYW55DQo+IHJlYXNvbiB0byBzZXJpYWxpemUgSS9PIHRvIGRpZmZlcmVudCBmZHMgdGhlcmUu DQoNClNpbmNlIHRoZXJlIGlzIG9ubHkgMSBtYWluIHRocmVhZCB0aGF0IHJlYWRzL2NyZWF0ZXMg dGhlIGV2ZW50IGZvciB0aGUgdGhyZWFkIHRvIGNvbnN1bWUgSSBkb27igJl0IHNlZSBob3cgd2Ug Y2FuIGhhdmUgbXVsdGlwbGUgdGhyZWFkcyBzdGFydGVkIHdpdGggZGlmZmVyZW50IGZkcy4gSnVz dCB0byBjbGFyaWZ5LCBtYWluIHRocmVhZCBvbmx5IHdhaXRzIGZvciB0aGUgdXBjYWxsIGluZm8g dG8gYmUgY29uc3VtZWQgZnJvbSB0aGUgZmQgbm90IGZvciB0aGUgcHJvY2VzcyBvZiB0aGUgdXBj YWxsLiBUaGVuLCBtdWx0aXBsZSB1cGNhbGxzIGNhbiBiZSBwcm9jZXNzaW5nIGF0IHRoZSBzYW1l IHRpbWUuDQoNCg0KPiANCj4gUmV2aWV3ZWQtYnk6IEplZmYgTGF5dG9uIDxqbGF5dG9uQHBvb2No aWVyZWRzLm5ldD4NCg0K ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-28 14:07 ` Kornievskaia, Olga @ 2016-04-30 12:29 ` Jeff Layton 2016-04-30 16:52 ` Olga Kornievskaia [not found] ` <BF14CEF3-6B57-43FD-9226-00A25886844F@netapp.com> 0 siblings, 2 replies; 16+ messages in thread From: Jeff Layton @ 2016-04-30 12:29 UTC (permalink / raw) To: Kornievskaia, Olga; +Cc: steved, linux-nfs On Thu, 2016-04-28 at 14:07 +0000, Kornievskaia, Olga wrote: > > > > On Apr 28, 2016, at 9:13 AM, Jeff Layton <jlayton@poochiereds.net> > > wrote: > > > > On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: > > > > > > Currently, to persevere global data over multiple mounts, > > > the root process does not fork when handling an upcall. > > > Instead on not-forking create a pthread to handle the > > > upcall since global data can be shared among threads. > > > > > > Signed-off-by: Steve Dickson <steved@redhat.com> > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > > --- > > > aclocal/libpthread.m4 | 13 +++++++++++++ > > > configure.ac | 3 +++ > > > utils/gssd/Makefile.am | 3 ++- > > > utils/gssd/gssd.c | 45 > > > ++++++++++++++++++++++++++++++++++++++++----- > > > utils/gssd/gssd.h | 5 +++++ > > > utils/gssd/gssd_proc.c | 49 ++++++++++++++++++---------------- > > > --------------- > > > utils/gssd/krb5_util.c | 3 +++ > > > 7 files changed, 84 insertions(+), 37 deletions(-) > > > create mode 100644 aclocal/libpthread.m4 > > > > > > diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 > > > new file mode 100644 > > > index 0000000..e87d2a0 > > > --- /dev/null > > > +++ b/aclocal/libpthread.m4 > > > @@ -0,0 +1,13 @@ > > > +dnl Checks for pthreads library and headers > > > +dnl > > > +AC_DEFUN([AC_LIBPTHREAD], [ > > > + > > > + dnl Check for library, but do not add -lpthreads to LIBS > > > + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=- > > > lpthread], > > > + [AC_MSG_ERROR([libpthread not found.])]) > > > + AC_SUBST(LIBPTHREAD) > > > + > > > + AC_CHECK_HEADERS([pthread.h], , > > > + [AC_MSG_ERROR([libpthread headers not > > > found.])]) > > > + > > > +])dnl > > > diff --git a/configure.ac b/configure.ac > > > index 25d2ba4..e0ecd61 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then > > > dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to > > > have KRBLIBS set > > > AC_LIBRPCSECGSS > > > > > > + dnl Check for pthreads > > > + AC_LIBPTHREAD > > > + > > > dnl librpcsecgss already has a dependency on libgssapi, > > > dnl but we need to make sure we get the right version > > > if test "$enable_gss" = yes; then > > > diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am > > > index cb040b3..3f5f59a 100644 > > > --- a/utils/gssd/Makefile.am > > > +++ b/utils/gssd/Makefile.am > > > @@ -49,7 +49,8 @@ gssd_LDADD = \ > > > $(RPCSECGSS_LIBS) \ > > > $(KRBLIBS) \ > > > $(GSSAPI_LIBS) \ > > > - $(LIBTIRPC) > > > + $(LIBTIRPC) \ > > > + $(LIBPTHREAD) > > > > > > gssd_LDFLAGS = \ > > > $(KRBLDFLAGS) > > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > > > index e7cb07f..b676341 100644 > > > --- a/utils/gssd/gssd.c > > > +++ b/utils/gssd/gssd.c > > > @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; > > > char *preferred_realm = NULL; > > > /* Avoid DNS reverse lookups on server names */ > > > static bool avoid_dns = true; > > > - > > > +int thread_started = false; > > > +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; > > > +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; > > > > > > TAILQ_HEAD(topdir_list_head, topdir) topdir_list; > > > > > > @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info *clp) > > > > > > static void gssd_scan(void); > > > > > > +static inline void wait_for_child_and_detach(pthread_t th) > > > +{ > > > + pthread_mutex_lock(&pmutex); > > > + while (!thread_started) > > > + pthread_cond_wait(&pcond, &pmutex); > > > + thread_started = false; > > > + pthread_mutex_unlock(&pmutex); > > > + pthread_detach(th); > > > +} > > > + > > > +/* For each upcall create a thread, detach from the main process > > > so that > > > + * resources are released back into the system without the need > > > for a join. > > > + * We need to wait for the child thread to start and consume the > > > event from > > > + * the file descriptor. > > > + */ > > > static void > > > gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void > > > *data) > > > { > > > struct clnt_info *clp = data; > > > - > > > - handle_gssd_upcall(clp); > > > + pthread_t th; > > > + int ret; > > > + > > > + ret = pthread_create(&th, NULL, (void > > > *)handle_gssd_upcall, > > > + (void *)clp); > > > + if (ret != 0) { > > > + printerr(0, "ERROR: pthread_create failed: ret > > > %d: %s\n", > > > + ret, strerror(errno)); > > > + return; > > > + } > > > + wait_for_child_and_detach(th); > > > } > > > > > > static void > > > gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void > > > *data) > > > { > > > struct clnt_info *clp = data; > > > - > > > - handle_krb5_upcall(clp); > > > + pthread_t th; > > > + int ret; > > > + > > > + ret = pthread_create(&th, NULL, (void > > > *)handle_krb5_upcall, > > > + (void *)clp); > > > + if (ret != 0) { > > > + printerr(0, "ERROR: pthread_create failed: ret > > > %d: %s\n", > > > + ret, strerror(errno)); > > > + return; > > > + } > > > + wait_for_child_and_detach(th); > > > } > > > > > > static struct clnt_info * > > > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > > > index c6937c5..565bce3 100644 > > > --- a/utils/gssd/gssd.h > > > +++ b/utils/gssd/gssd.h > > > @@ -36,6 +36,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > > > > #ifndef GSSD_PIPEFS_DIR > > > #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" > > > @@ -61,6 +62,10 @@ extern int root_uses_ma > > > chine_creds; > > > extern unsigned int context_timeout; > > > extern unsigned int rpc_timeout; > > > extern char *preferred_realm; > > > +extern pthread_mutex_t ple_lock; > > > +extern pthread_cond_t pcond; > > > +extern pthread_mutex_t pmutex; > > > +extern int thread_started; > > > > > > struct clnt_info { > > > TAILQ_ENTRY(clnt_info) list; > > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > > > index 1ef68d8..e2e95dc 100644 > > > --- a/utils/gssd/gssd_proc.c > > > +++ b/utils/gssd/gssd_proc.c > > > @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, > > > uid_t uid, int fd, char *tgtname, > > > if (uid != 0 || (uid == 0 && root_uses_machine_creds == > > > 0 && > > > service == NULL)) { > > > > > > - /* already running as uid 0 */ > > > - if (uid == 0) > > > - goto no_fork; > > > - > > > - pid = fork(); > > > - switch(pid) { > > > - case 0: > > > - /* Child: fall through to rest of > > > function */ > > > - childpid = getpid(); > > > - unsetenv("KRB5CCNAME"); > > > - printerr(2, "CHILD forked pid %d \n", > > > childpid); > > > - break; > > > - case -1: > > > - /* fork() failed! */ > > > - printerr(0, "WARNING: unable to fork() > > > to handle" > > > - "upcall: %s\n", > > > strerror(errno)); > > > - return; > > > - default: > > > - /* Parent: just wait on child to exit > > > and return */ > > > - do { > > > - pid = wait(&err); > > > - } while(pid == -1 && errno != -ECHILD); > > > - > > > - if (WIFSIGNALED(err)) > > > - printerr(0, "WARNING: forked > > > child was killed" > > > - "with signal %d\n", > > > WTERMSIG(err)); > > > - return; > > > - } > > > -no_fork: > > > - > > > auth = krb5_not_machine_creds(clp, uid, tgtname, > > > &downcall_err, > > > &err, > > > &rpc_clnt); > > > if (err) > > > @@ -735,12 +705,28 @@ out_return_error: > > > goto out; > > > } > > > > > > +/* signal to the parent thread that we have read from the file > > > descriptor. > > > + * it should allow the parent to proceed to poll on the > > > descriptor for > > > + * the next upcall from the kernel. > > > + */ > > > +static void > > > +signal_parent_event_consumed(void) > > > +{ > > > + pthread_mutex_lock(&pmutex); > > > + thread_started = true; > > > + pthread_cond_signal(&pcond); > > > + pthread_mutex_unlock(&pmutex); > > > +} > > > + > > > void > > > handle_krb5_upcall(struct clnt_info *clp) > > > { > > > uid_t uid; > > > + int status; > > > > > > - if (read(clp->krb5_fd, &uid, sizeof(uid)) < > > > (ssize_t)sizeof(uid)) { > > > + status = read(clp->krb5_fd, &uid, sizeof(uid)) < > > > (ssize_t)sizeof(uid); > > > + signal_parent_event_consumed(); > > > + if (status) { > > > printerr(0, "WARNING: failed reading uid from > > > krb5 " > > > "upcall pipe: %s\n", > > > strerror(errno)); > > > return; > > > @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) > > > char *enctypes = NULL; > > > > > > lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); > > > + signal_parent_event_consumed(); > > > if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { > > > printerr(0, "WARNING: handle_gssd_upcall: " > > > "failed reading request\n"); > > > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > > > index 8ef8184..3328696 100644 > > > --- a/utils/gssd/krb5_util.c > > > +++ b/utils/gssd/krb5_util.c > > > @@ -128,6 +128,7 @@ > > > > > > /* Global list of principals/cache file names for machine > > > credentials */ > > > struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; > > > +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; > > > > > > #ifdef HAVE_SET_ALLOWABLE_ENCTYPES > > > int limit_to_legacy_enctypes = 0; > > > @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, > > > krb5_principal princ) > > > > > > /* Need to serialize list if we ever become multi- > > > threaded! */ > > > > > > + pthread_mutex_lock(&ple_lock); > > > ple = find_ple_by_princ(context, princ); > > > if (ple == NULL) { > > > ple = new_ple(context, princ); > > > } > > > + pthread_mutex_unlock(&ple_lock); > > > > > > return ple; > > > } > > Looks good. One thing that might be neat as a follow up is to make > > the > > thread_started variable a per-clnt thing. I don't think there's any > > reason to serialize I/O to different fds there. > Since there is only 1 main thread that reads/creates the event for > the thread to consume I don’t see how we can have multiple threads > started with different fds. Just to clarify, main thread only waits > for the upcall info to be consumed from the fd not for the process of > the upcall. Then, multiple upcalls can be processing at the same > time. > Ok, yeah you do need to serialize there because of the event handling. You can't return from the event handler until the read event has been consumed or or it would get mighty confused. Still, I wonder -- might it be cleaner to do the read in the context of the event loop and then just spawn a detached thread to handle the result? It seems like that would be less back-and-forth between the parent and child. You also wouldn't need the pmutex and condition variable with that. You would have to come up with some way to pass both "clp" and "uid" to the child however. Either way, that could be reworked later and this is a clear improvement. Nice work, btw! -- Jeff Layton <jlayton@poochiereds.net> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls 2016-04-30 12:29 ` Jeff Layton @ 2016-04-30 16:52 ` Olga Kornievskaia [not found] ` <BF14CEF3-6B57-43FD-9226-00A25886844F@netapp.com> 1 sibling, 0 replies; 16+ messages in thread From: Olga Kornievskaia @ 2016-04-30 16:52 UTC (permalink / raw) To: linux-nfs On Sat, Apr 30, 2016 at 8:29 AM, Jeff Layton <jlayton@poochiereds.net> wrote: > On Thu, 2016-04-28 at 14:07 +0000, Kornievskaia, Olga wrote: >> > >> > On Apr 28, 2016, at 9:13 AM, Jeff Layton <jlayton@poochiereds.net> >> > wrote: >> > >> > On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: >> > > >> > > Currently, to persevere global data over multiple mounts, >> > > the root process does not fork when handling an upcall. >> > > Instead on not-forking create a pthread to handle the >> > > upcall since global data can be shared among threads. >> > > >> > > Signed-off-by: Steve Dickson <steved@redhat.com> >> > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> >> > > --- >> > > aclocal/libpthread.m4 | 13 +++++++++++++ >> > > configure.ac | 3 +++ >> > > utils/gssd/Makefile.am | 3 ++- >> > > utils/gssd/gssd.c | 45 >> > > ++++++++++++++++++++++++++++++++++++++++----- >> > > utils/gssd/gssd.h | 5 +++++ >> > > utils/gssd/gssd_proc.c | 49 ++++++++++++++++++---------------- >> > > --------------- >> > > utils/gssd/krb5_util.c | 3 +++ >> > > 7 files changed, 84 insertions(+), 37 deletions(-) >> > > create mode 100644 aclocal/libpthread.m4 >> > > >> > > diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 >> > > new file mode 100644 >> > > index 0000000..e87d2a0 >> > > --- /dev/null >> > > +++ b/aclocal/libpthread.m4 >> > > @@ -0,0 +1,13 @@ >> > > +dnl Checks for pthreads library and headers >> > > +dnl >> > > +AC_DEFUN([AC_LIBPTHREAD], [ >> > > + >> > > + dnl Check for library, but do not add -lpthreads to LIBS >> > > + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=- >> > > lpthread], >> > > + [AC_MSG_ERROR([libpthread not found.])]) >> > > + AC_SUBST(LIBPTHREAD) >> > > + >> > > + AC_CHECK_HEADERS([pthread.h], , >> > > + [AC_MSG_ERROR([libpthread headers not >> > > found.])]) >> > > + >> > > +])dnl >> > > diff --git a/configure.ac b/configure.ac >> > > index 25d2ba4..e0ecd61 100644 >> > > --- a/configure.ac >> > > +++ b/configure.ac >> > > @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then >> > > dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to >> > > have KRBLIBS set >> > > AC_LIBRPCSECGSS >> > > >> > > + dnl Check for pthreads >> > > + AC_LIBPTHREAD >> > > + >> > > dnl librpcsecgss already has a dependency on libgssapi, >> > > dnl but we need to make sure we get the right version >> > > if test "$enable_gss" = yes; then >> > > diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am >> > > index cb040b3..3f5f59a 100644 >> > > --- a/utils/gssd/Makefile.am >> > > +++ b/utils/gssd/Makefile.am >> > > @@ -49,7 +49,8 @@ gssd_LDADD = \ >> > > $(RPCSECGSS_LIBS) \ >> > > $(KRBLIBS) \ >> > > $(GSSAPI_LIBS) \ >> > > - $(LIBTIRPC) >> > > + $(LIBTIRPC) \ >> > > + $(LIBPTHREAD) >> > > >> > > gssd_LDFLAGS = \ >> > > $(KRBLDFLAGS) >> > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c >> > > index e7cb07f..b676341 100644 >> > > --- a/utils/gssd/gssd.c >> > > +++ b/utils/gssd/gssd.c >> > > @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; >> > > char *preferred_realm = NULL; >> > > /* Avoid DNS reverse lookups on server names */ >> > > static bool avoid_dns = true; >> > > - >> > > +int thread_started = false; >> > > +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; >> > > +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; >> > > >> > > TAILQ_HEAD(topdir_list_head, topdir) topdir_list; >> > > >> > > @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info *clp) >> > > >> > > static void gssd_scan(void); >> > > >> > > +static inline void wait_for_child_and_detach(pthread_t th) >> > > +{ >> > > + pthread_mutex_lock(&pmutex); >> > > + while (!thread_started) >> > > + pthread_cond_wait(&pcond, &pmutex); >> > > + thread_started = false; >> > > + pthread_mutex_unlock(&pmutex); >> > > + pthread_detach(th); >> > > +} >> > > + >> > > +/* For each upcall create a thread, detach from the main process >> > > so that >> > > + * resources are released back into the system without the need >> > > for a join. >> > > + * We need to wait for the child thread to start and consume the >> > > event from >> > > + * the file descriptor. >> > > + */ >> > > static void >> > > gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void >> > > *data) >> > > { >> > > struct clnt_info *clp = data; >> > > - >> > > - handle_gssd_upcall(clp); >> > > + pthread_t th; >> > > + int ret; >> > > + >> > > + ret = pthread_create(&th, NULL, (void >> > > *)handle_gssd_upcall, >> > > + (void *)clp); >> > > + if (ret != 0) { >> > > + printerr(0, "ERROR: pthread_create failed: ret >> > > %d: %s\n", >> > > + ret, strerror(errno)); >> > > + return; >> > > + } >> > > + wait_for_child_and_detach(th); >> > > } >> > > >> > > static void >> > > gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void >> > > *data) >> > > { >> > > struct clnt_info *clp = data; >> > > - >> > > - handle_krb5_upcall(clp); >> > > + pthread_t th; >> > > + int ret; >> > > + >> > > + ret = pthread_create(&th, NULL, (void >> > > *)handle_krb5_upcall, >> > > + (void *)clp); >> > > + if (ret != 0) { >> > > + printerr(0, "ERROR: pthread_create failed: ret >> > > %d: %s\n", >> > > + ret, strerror(errno)); >> > > + return; >> > > + } >> > > + wait_for_child_and_detach(th); >> > > } >> > > >> > > static struct clnt_info * >> > > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h >> > > index c6937c5..565bce3 100644 >> > > --- a/utils/gssd/gssd.h >> > > +++ b/utils/gssd/gssd.h >> > > @@ -36,6 +36,7 @@ >> > > #include >> > > #include >> > > #include >> > > +#include >> > > >> > > #ifndef GSSD_PIPEFS_DIR >> > > #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" >> > > @@ -61,6 +62,10 @@ extern int root_uses_ma >> > > chine_creds; >> > > extern unsigned int context_timeout; >> > > extern unsigned int rpc_timeout; >> > > extern char *preferred_realm; >> > > +extern pthread_mutex_t ple_lock; >> > > +extern pthread_cond_t pcond; >> > > +extern pthread_mutex_t pmutex; >> > > +extern int thread_started; >> > > >> > > struct clnt_info { >> > > TAILQ_ENTRY(clnt_info) list; >> > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c >> > > index 1ef68d8..e2e95dc 100644 >> > > --- a/utils/gssd/gssd_proc.c >> > > +++ b/utils/gssd/gssd_proc.c >> > > @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, >> > > uid_t uid, int fd, char *tgtname, >> > > if (uid != 0 || (uid == 0 && root_uses_machine_creds == >> > > 0 && >> > > service == NULL)) { >> > > >> > > - /* already running as uid 0 */ >> > > - if (uid == 0) >> > > - goto no_fork; >> > > - >> > > - pid = fork(); >> > > - switch(pid) { >> > > - case 0: >> > > - /* Child: fall through to rest of >> > > function */ >> > > - childpid = getpid(); >> > > - unsetenv("KRB5CCNAME"); >> > > - printerr(2, "CHILD forked pid %d \n", >> > > childpid); >> > > - break; >> > > - case -1: >> > > - /* fork() failed! */ >> > > - printerr(0, "WARNING: unable to fork() >> > > to handle" >> > > - "upcall: %s\n", >> > > strerror(errno)); >> > > - return; >> > > - default: >> > > - /* Parent: just wait on child to exit >> > > and return */ >> > > - do { >> > > - pid = wait(&err); >> > > - } while(pid == -1 && errno != -ECHILD); >> > > - >> > > - if (WIFSIGNALED(err)) >> > > - printerr(0, "WARNING: forked >> > > child was killed" >> > > - "with signal %d\n", >> > > WTERMSIG(err)); >> > > - return; >> > > - } >> > > -no_fork: >> > > - >> > > auth = krb5_not_machine_creds(clp, uid, tgtname, >> > > &downcall_err, >> > > &err, >> > > &rpc_clnt); >> > > if (err) >> > > @@ -735,12 +705,28 @@ out_return_error: >> > > goto out; >> > > } >> > > >> > > +/* signal to the parent thread that we have read from the file >> > > descriptor. >> > > + * it should allow the parent to proceed to poll on the >> > > descriptor for >> > > + * the next upcall from the kernel. >> > > + */ >> > > +static void >> > > +signal_parent_event_consumed(void) >> > > +{ >> > > + pthread_mutex_lock(&pmutex); >> > > + thread_started = true; >> > > + pthread_cond_signal(&pcond); >> > > + pthread_mutex_unlock(&pmutex); >> > > +} >> > > + >> > > void >> > > handle_krb5_upcall(struct clnt_info *clp) >> > > { >> > > uid_t uid; >> > > + int status; >> > > >> > > - if (read(clp->krb5_fd, &uid, sizeof(uid)) < >> > > (ssize_t)sizeof(uid)) { >> > > + status = read(clp->krb5_fd, &uid, sizeof(uid)) < >> > > (ssize_t)sizeof(uid); >> > > + signal_parent_event_consumed(); >> > > + if (status) { >> > > printerr(0, "WARNING: failed reading uid from >> > > krb5 " >> > > "upcall pipe: %s\n", >> > > strerror(errno)); >> > > return; >> > > @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) >> > > char *enctypes = NULL; >> > > >> > > lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); >> > > + signal_parent_event_consumed(); >> > > if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { >> > > printerr(0, "WARNING: handle_gssd_upcall: " >> > > "failed reading request\n"); >> > > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c >> > > index 8ef8184..3328696 100644 >> > > --- a/utils/gssd/krb5_util.c >> > > +++ b/utils/gssd/krb5_util.c >> > > @@ -128,6 +128,7 @@ >> > > >> > > /* Global list of principals/cache file names for machine >> > > credentials */ >> > > struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; >> > > +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; >> > > >> > > #ifdef HAVE_SET_ALLOWABLE_ENCTYPES >> > > int limit_to_legacy_enctypes = 0; >> > > @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, >> > > krb5_principal princ) >> > > >> > > /* Need to serialize list if we ever become multi- >> > > threaded! */ >> > > >> > > + pthread_mutex_lock(&ple_lock); >> > > ple = find_ple_by_princ(context, princ); >> > > if (ple == NULL) { >> > > ple = new_ple(context, princ); >> > > } >> > > + pthread_mutex_unlock(&ple_lock); >> > > >> > > return ple; >> > > } >> > Looks good. One thing that might be neat as a follow up is to make >> > the >> > thread_started variable a per-clnt thing. I don't think there's any >> > reason to serialize I/O to different fds there. >> Since there is only 1 main thread that reads/creates the event for >> the thread to consume I don’t see how we can have multiple threads >> started with different fds. Just to clarify, main thread only waits >> for the upcall info to be consumed from the fd not for the process of >> the upcall. Then, multiple upcalls can be processing at the same >> time. >> > > Ok, yeah you do need to serialize there because of the event handling. > You can't return from the event handler until the read event has been > consumed or or it would get mighty confused. > > Still, I wonder -- might it be cleaner to do the read in the context of > the event loop and then just spawn a detached thread to handle the > result? > > It seems like that would be less back-and-forth between the parent and > child. You also wouldn't need the pmutex and condition variable with > that. You would have to come up with some way to pass both "clp" and > "uid" to the child however. > > Either way, that could be reworked later and this is a clear > improvement. Nice work, btw! I think that’s a good idea, thanks. Are you suggesting to read and parse in the main thread (the uid as well as other info is known after parsing)? Or we can do the read() and pass the buffer for the child to parse. Instead of doing conditioned wait the main thread need to do a bit more work --read + memory allocation. Right now child thread has static memory for the read buffer. With a pool of threads I think we’d need to do the read in the main thread anyway. ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <BF14CEF3-6B57-43FD-9226-00A25886844F@netapp.com>]
* Re: [PATCH v5 1/3] gssd: use pthreads to handle upcalls [not found] ` <BF14CEF3-6B57-43FD-9226-00A25886844F@netapp.com> @ 2016-04-30 17:29 ` Jeff Layton 0 siblings, 0 replies; 16+ messages in thread From: Jeff Layton @ 2016-04-30 17:29 UTC (permalink / raw) To: Kornievskaia, Olga; +Cc: steved, linux-nfs On Sat, 2016-04-30 at 15:16 +0000, Kornievskaia, Olga wrote: > > > On Apr 30, 2016, at 8:29 AM, Jeff Layton <jlayton@poochiereds.net> > > wrote: > > > > On Thu, 2016-04-28 at 14:07 +0000, Kornievskaia, Olga wrote: > > > > > > > > On Apr 28, 2016, at 9:13 AM, Jeff Layton <jlayton@poochiereds.n > > > > et> > > > > wrote: > > > > > > > > On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: > > > > > > > > > > Currently, to persevere global data over multiple mounts, > > > > > the root process does not fork when handling an upcall. > > > > > Instead on not-forking create a pthread to handle the > > > > > upcall since global data can be shared among threads. > > > > > > > > > > Signed-off-by: Steve Dickson <steved@redhat.com> > > > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > > > > --- > > > > > aclocal/libpthread.m4 | 13 +++++++++++++ > > > > > configure.ac | 3 +++ > > > > > utils/gssd/Makefile.am | 3 ++- > > > > > utils/gssd/gssd.c | 45 > > > > > ++++++++++++++++++++++++++++++++++++++++----- > > > > > utils/gssd/gssd.h | 5 +++++ > > > > > utils/gssd/gssd_proc.c | 49 ++++++++++++++++++------------ > > > > > ---- > > > > > --------------- > > > > > utils/gssd/krb5_util.c | 3 +++ > > > > > 7 files changed, 84 insertions(+), 37 deletions(-) > > > > > create mode 100644 aclocal/libpthread.m4 > > > > > > > > > > diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4 > > > > > new file mode 100644 > > > > > index 0000000..e87d2a0 > > > > > --- /dev/null > > > > > +++ b/aclocal/libpthread.m4 > > > > > @@ -0,0 +1,13 @@ > > > > > +dnl Checks for pthreads library and headers > > > > > +dnl > > > > > +AC_DEFUN([AC_LIBPTHREAD], [ > > > > > + > > > > > + dnl Check for library, but do not add -lpthreads to LIBS > > > > > + AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=- > > > > > lpthread], > > > > > + [AC_MSG_ERROR([libpthread not found.])]) > > > > > + AC_SUBST(LIBPTHREAD) > > > > > + > > > > > + AC_CHECK_HEADERS([pthread.h], , > > > > > + [AC_MSG_ERROR([libpthread headers not > > > > > found.])]) > > > > > + > > > > > +])dnl > > > > > diff --git a/configure.ac b/configure.ac > > > > > index 25d2ba4..e0ecd61 100644 > > > > > --- a/configure.ac > > > > > +++ b/configure.ac > > > > > @@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then > > > > > dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to > > > > > have KRBLIBS set > > > > > AC_LIBRPCSECGSS > > > > > > > > > > + dnl Check for pthreads > > > > > + AC_LIBPTHREAD > > > > > + > > > > > dnl librpcsecgss already has a dependency on libgssapi, > > > > > dnl but we need to make sure we get the right version > > > > > if test "$enable_gss" = yes; then > > > > > diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am > > > > > index cb040b3..3f5f59a 100644 > > > > > --- a/utils/gssd/Makefile.am > > > > > +++ b/utils/gssd/Makefile.am > > > > > @@ -49,7 +49,8 @@ gssd_LDADD = \ > > > > > $(RPCSECGSS_LIBS) \ > > > > > $(KRBLIBS) \ > > > > > $(GSSAPI_LIBS) \ > > > > > - $(LIBTIRPC) > > > > > + $(LIBTIRPC) \ > > > > > + $(LIBPTHREAD) > > > > > > > > > > gssd_LDFLAGS = \ > > > > > $(KRBLDFLAGS) > > > > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > > > > > index e7cb07f..b676341 100644 > > > > > --- a/utils/gssd/gssd.c > > > > > +++ b/utils/gssd/gssd.c > > > > > @@ -87,7 +87,9 @@ unsigned int rpc_timeout = 5; > > > > > char *preferred_realm = NULL; > > > > > /* Avoid DNS reverse lookups on server names */ > > > > > static bool avoid_dns = true; > > > > > - > > > > > +int thread_started = false; > > > > > +pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER; > > > > > +pthread_cond_t pcond = PTHREAD_COND_INITIALIZER; > > > > > > > > > > TAILQ_HEAD(topdir_list_head, topdir) topdir_list; > > > > > > > > > > @@ -361,20 +363,53 @@ gssd_destroy_client(struct clnt_info > > > > > *clp) > > > > > > > > > > static void gssd_scan(void); > > > > > > > > > > +static inline void wait_for_child_and_detach(pthread_t th) > > > > > +{ > > > > > + pthread_mutex_lock(&pmutex); > > > > > + while (!thread_started) > > > > > + pthread_cond_wait(&pcond, &pmutex); > > > > > + thread_started = false; > > > > > + pthread_mutex_unlock(&pmutex); > > > > > + pthread_detach(th); > > > > > +} > > > > > + Another relatively minor nit as well: Since you know that you're always going to detach these threads, it's probably more efficient to just create them in an already-detached state (a'la pthread_attr_setdetachstate). AIUI, then the thread spawning code can skip setting up the stuff to allow joins. > > > > > +/* For each upcall create a thread, detach from the main > > > > > process > > > > > so that > > > > > + * resources are released back into the system without the > > > > > need > > > > > for a join. > > > > > + * We need to wait for the child thread to start and consume > > > > > the > > > > > event from > > > > > + * the file descriptor. > > > > > + */ > > > > > static void > > > > > gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void > > > > > *data) > > > > > { > > > > > struct clnt_info *clp = data; > > > > > - > > > > > - handle_gssd_upcall(clp); > > > > > + pthread_t th; > > > > > + int ret; > > > > > + > > > > > + ret = pthread_create(&th, NULL, (void > > > > > *)handle_gssd_upcall, > > > > > + (void *)clp); > > > > > + if (ret != 0) { > > > > > + printerr(0, "ERROR: pthread_create failed: ret > > > > > %d: %s\n", > > > > > + ret, strerror(errno)); > > > > > + return; > > > > > + } > > > > > + wait_for_child_and_detach(th); > > > > > } > > > > > > > > > > static void > > > > > gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void > > > > > *data) > > > > > { > > > > > struct clnt_info *clp = data; > > > > > - > > > > > - handle_krb5_upcall(clp); > > > > > + pthread_t th; > > > > > + int ret; > > > > > + > > > > > + ret = pthread_create(&th, NULL, (void > > > > > *)handle_krb5_upcall, > > > > > + (void *)clp); > > > > > + if (ret != 0) { > > > > > + printerr(0, "ERROR: pthread_create failed: ret > > > > > %d: %s\n", > > > > > + ret, strerror(errno)); > > > > > + return; > > > > > + } > > > > > + wait_for_child_and_detach(th); > > > > > } > > > > > > > > > > static struct clnt_info * > > > > > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > > > > > index c6937c5..565bce3 100644 > > > > > --- a/utils/gssd/gssd.h > > > > > +++ b/utils/gssd/gssd.h > > > > > @@ -36,6 +36,7 @@ > > > > > #include > > > > > #include > > > > > #include > > > > > +#include > > > > > > > > > > #ifndef GSSD_PIPEFS_DIR > > > > > #define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs" > > > > > @@ -61,6 +62,10 @@ extern int > > > > > root_uses_ma > > > > > chine_creds; > > > > > extern unsigned int context_timeout; > > > > > extern unsigned int rpc_timeout; > > > > > extern char *preferred_realm; > > > > > +extern pthread_mutex_t ple_lock; > > > > > +extern pthread_cond_t pcond; > > > > > +extern pthread_mutex_t pmutex; > > > > > +extern int thread_started; > > > > > > > > > > struct clnt_info { > > > > > TAILQ_ENTRY(clnt_info) > > > > > list; > > > > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > > > > > index 1ef68d8..e2e95dc 100644 > > > > > --- a/utils/gssd/gssd_proc.c > > > > > +++ b/utils/gssd/gssd_proc.c > > > > > @@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info > > > > > *clp, > > > > > uid_t uid, int fd, char *tgtname, > > > > > if (uid != 0 || (uid == 0 && root_uses_machine_creds == > > > > > 0 && > > > > > service == NULL)) { > > > > > > > > > > - /* already running as uid 0 */ > > > > > - if (uid == 0) > > > > > - goto no_fork; > > > > > - > > > > > - pid = fork(); > > > > > - switch(pid) { > > > > > - case 0: > > > > > - /* Child: fall through to rest of > > > > > function */ > > > > > - childpid = getpid(); > > > > > - unsetenv("KRB5CCNAME"); > > > > > - printerr(2, "CHILD forked pid %d \n", > > > > > childpid); > > > > > - break; > > > > > - case -1: > > > > > - /* fork() failed! */ > > > > > - printerr(0, "WARNING: unable to fork() > > > > > to handle" > > > > > - "upcall: %s\n", > > > > > strerror(errno)); > > > > > - return; > > > > > - default: > > > > > - /* Parent: just wait on child to exit > > > > > and return */ > > > > > - do { > > > > > - pid = wait(&err); > > > > > - } while(pid == -1 && errno != -ECHILD); > > > > > - > > > > > - if (WIFSIGNALED(err)) > > > > > - printerr(0, "WARNING: forked > > > > > child was killed" > > > > > - "with signal %d\n", > > > > > WTERMSIG(err)); > > > > > - return; > > > > > - } > > > > > -no_fork: > > > > > - > > > > > auth = krb5_not_machine_creds(clp, uid, tgtname, > > > > > &downcall_err, > > > > > &err, > > > > > &rpc_clnt); > > > > > if (err) > > > > > @@ -735,12 +705,28 @@ out_return_error: > > > > > goto out; > > > > > } > > > > > > > > > > +/* signal to the parent thread that we have read from the > > > > > file > > > > > descriptor. > > > > > + * it should allow the parent to proceed to poll on the > > > > > descriptor for > > > > > + * the next upcall from the kernel. > > > > > + */ > > > > > +static void > > > > > +signal_parent_event_consumed(void) > > > > > +{ > > > > > + pthread_mutex_lock(&pmutex); > > > > > + thread_started = true; > > > > > + pthread_cond_signal(&pcond); > > > > > + pthread_mutex_unlock(&pmutex); > > > > > +} > > > > > + > > > > > void > > > > > handle_krb5_upcall(struct clnt_info *clp) > > > > > { > > > > > uid_t > > > > > uid; > > > > > + int > > > > > status; > > > > > > > > > > - if (read(clp->krb5_fd, &uid, sizeof(uid)) < > > > > > (ssize_t)sizeof(uid)) { > > > > > + status = read(clp->krb5_fd, &uid, sizeof(uid)) < > > > > > (ssize_t)sizeof(uid); > > > > > + signal_parent_event_consumed(); > > > > > + if (status) { > > > > > printerr(0, "WARNING: failed reading uid from > > > > > krb5 " > > > > > "upcall pipe: %s\n", > > > > > strerror(errno)); > > > > > return; > > > > > @@ -765,6 +751,7 @@ handle_gssd_upcall(struct clnt_info *clp) > > > > > char > > > > > *enctypes = NULL; > > > > > > > > > > lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf)); > > > > > + signal_parent_event_consumed(); > > > > > if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') { > > > > > printerr(0, "WARNING: handle_gssd_upcall: " > > > > > "failed reading request\n"); > > > > > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > > > > > index 8ef8184..3328696 100644 > > > > > --- a/utils/gssd/krb5_util.c > > > > > +++ b/utils/gssd/krb5_util.c > > > > > @@ -128,6 +128,7 @@ > > > > > > > > > > /* Global list of principals/cache file names for machine > > > > > credentials */ > > > > > struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL; > > > > > +pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER; > > > > > > > > > > #ifdef HAVE_SET_ALLOWABLE_ENCTYPES > > > > > int limit_to_legacy_enctypes = 0; > > > > > @@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, > > > > > krb5_principal princ) > > > > > > > > > > /* Need to serialize list if we ever become multi- > > > > > threaded! */ > > > > > > > > > > + pthread_mutex_lock(&ple_lock); > > > > > ple = find_ple_by_princ(context, princ); > > > > > if (ple == NULL) { > > > > > ple = new_ple(context, princ); > > > > > } > > > > > + pthread_mutex_unlock(&ple_lock); > > > > > > > > > > return ple; > > > > > } > > > > Looks good. One thing that might be neat as a follow up is to > > > > make > > > > the > > > > thread_started variable a per-clnt thing. I don't think there's > > > > any > > > > reason to serialize I/O to different fds there. > > > Since there is only 1 main thread that reads/creates the event > > > for > > > the thread to consume I don’t see how we can have multiple > > > threads > > > started with different fds. Just to clarify, main thread only > > > waits > > > for the upcall info to be consumed from the fd not for the > > > process of > > > the upcall. Then, multiple upcalls can be processing at the same > > > time. > > > > > > > Ok, yeah you do need to serialize there because of the event > > handling. > > You can't return from the event handler until the read event has > > been > > consumed or or it would get mighty confused. > > > > Still, I wonder -- might it be cleaner to do the read in the > > context of > > the event loop and then just spawn a detached thread to handle the > > result? > > It seems like that would be less back-and-forth between the parent > and > child. You also wouldn't need the pmutex and condition variable with > that. You would have to come up with some way to pass both "clp" and > "uid" to the child however. > > Either way, that could be reworked later and this is a clear > improvement. Nice work, btw! > I think that’s a good idea, thanks. Are you suggesting to read and > parse in the main thread (the uid as well as other info is known > after parsing)? Or we can do the read() and pass the buffer for the > child to parse. Instead of doing conditioned wait the main thread > need to do a bit more work --read + memory allocation. Right now > child thread has static memory for the read buffer. With a pool of > threads I think we’d need to do the read in the main thread anyway. > Yeah, the usual thing to do is to do the minimum amount that must be serialized in the context of the event loop (the read in this case) and then dispatch a thread to do the rest. I don't think you'd get away without doing an allocation here of some sort though if you go that route. Create a struct that has the clnt_info and uid and then pass that in the thread's arg. -- Jeff Layton <jlayton@poochiereds.net> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity 2016-04-27 16:58 [RFC PATCH v5 0/3] adding pthread support to gssd Olga Kornievskaia 2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia @ 2016-04-27 16:58 ` Olga Kornievskaia 2016-04-28 13:14 ` Jeff Layton 2016-04-27 16:58 ` [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name Olga Kornievskaia 2016-04-29 14:49 ` [RFC PATCH v5 0/3] adding pthread support to gssd Steve Dickson 3 siblings, 1 reply; 16+ messages in thread From: Olga Kornievskaia @ 2016-04-27 16:58 UTC (permalink / raw) To: steved; +Cc: linux-nfs For the threaded version we have to set uid,gid per thread instead of per process. glibc setresuid() when called from a thread, it'll send a signal to all other threads to synchronize the uid in all other threads. To bypass this, we have to call syscall() directly. Signed-off-by: Olga Kornievskaia <kolga@netapp.com> Reviewed-by: Steve Dickson <steved@redhat.com> --- utils/gssd/gssd_proc.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c index e2e95dc..de8dc07 100644 --- a/utils/gssd/gssd_proc.c +++ b/utils/gssd/gssd_proc.c @@ -69,6 +69,7 @@ #include <netdb.h> #include <sys/types.h> #include <sys/wait.h> +#include <syscall.h> #include "gssd.h" #include "err_util.h" @@ -436,7 +437,7 @@ change_identity(uid_t uid) struct passwd *pw; /* drop list of supplimentary groups first */ - if (setgroups(0, NULL) != 0) { + if (syscall(SYS_setgroups, 0, 0) != 0) { printerr(0, "WARNING: unable to drop supplimentary groups!"); return errno; } @@ -453,20 +454,18 @@ change_identity(uid_t uid) } } - /* - * Switch the GIDs. Note that we leave the saved-set-gid alone in an - * attempt to prevent attacks via ptrace() + /* Switch the UIDs and GIDs. */ + /* For the threaded version we have to set uid,gid per thread instead + * of per process. glibc setresuid() when called from a thread, it'll + * send a signal to all other threads to synchronize the uid in all + * other threads. To bypass this, we have to call syscall() directly. */ - if (setresgid(pw->pw_gid, pw->pw_gid, -1) != 0) { + if (syscall(SYS_setresgid, pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) { printerr(0, "WARNING: failed to set gid to %u!\n", pw->pw_gid); return errno; } - /* - * Switch UIDs, but leave saved-set-uid alone to prevent ptrace() by - * other processes running with this uid. - */ - if (setresuid(uid, uid, -1) != 0) { + if (syscall(SYS_setresuid, uid, uid, uid) != 0) { printerr(0, "WARNING: Failed to setuid for user with uid %u\n", uid); return errno; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity 2016-04-27 16:58 ` [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity Olga Kornievskaia @ 2016-04-28 13:14 ` Jeff Layton 0 siblings, 0 replies; 16+ messages in thread From: Jeff Layton @ 2016-04-28 13:14 UTC (permalink / raw) To: Olga Kornievskaia, steved; +Cc: linux-nfs On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: > For the threaded version we have to set uid,gid per thread instead > of per process. glibc setresuid() when called from a thread, it'll > send a signal to all other threads to synchronize the uid in all > other threads. To bypass this, we have to call syscall() directly. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > Reviewed-by: Steve Dickson <steved@redhat.com> > --- > utils/gssd/gssd_proc.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > index e2e95dc..de8dc07 100644 > --- a/utils/gssd/gssd_proc.c > +++ b/utils/gssd/gssd_proc.c > @@ -69,6 +69,7 @@ > #include > #include > #include > +#include > > #include "gssd.h" > #include "err_util.h" > @@ -436,7 +437,7 @@ change_identity(uid_t uid) > struct passwd *pw; > > /* drop list of supplimentary groups first */ > - if (setgroups(0, NULL) != 0) { > + if (syscall(SYS_setgroups, 0, 0) != 0) { > printerr(0, "WARNING: unable to drop supplimentary groups!"); > return errno; > } > @@ -453,20 +454,18 @@ change_identity(uid_t uid) > } > } > > - /* > - * Switch the GIDs. Note that we leave the saved-set-gid alone in an > - * attempt to prevent attacks via ptrace() > + /* Switch the UIDs and GIDs. */ > + /* For the threaded version we have to set uid,gid per thread instead > + * of per process. glibc setresuid() when called from a thread, it'll > + * send a signal to all other threads to synchronize the uid in all > + * other threads. To bypass this, we have to call syscall() directly. > */ > - if (setresgid(pw->pw_gid, pw->pw_gid, -1) != 0) { > + if (syscall(SYS_setresgid, pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) { > printerr(0, "WARNING: failed to set gid to %u!\n", pw->pw_gid); > return errno; > } > > - /* > - * Switch UIDs, but leave saved-set-uid alone to prevent ptrace() by > - * other processes running with this uid. > - */ > - if (setresuid(uid, uid, -1) != 0) { > + if (syscall(SYS_setresuid, uid, uid, uid) != 0) { > printerr(0, "WARNING: Failed to setuid for user with uid %u\n", > uid); > return errno; Reviewed-by: Jeff Layton <jlayton@poochiereds.net> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name 2016-04-27 16:58 [RFC PATCH v5 0/3] adding pthread support to gssd Olga Kornievskaia 2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia 2016-04-27 16:58 ` [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity Olga Kornievskaia @ 2016-04-27 16:58 ` Olga Kornievskaia 2016-04-28 13:19 ` Jeff Layton 2016-04-29 14:49 ` [RFC PATCH v5 0/3] adding pthread support to gssd Steve Dickson 3 siblings, 1 reply; 16+ messages in thread From: Olga Kornievskaia @ 2016-04-27 16:58 UTC (permalink / raw) To: steved; +Cc: linux-nfs Previously the location of the credential cache was passed in either using environment variable KRB5CCNAME or gss_krb5_ccache_name() if supported. For threaded-gssd, we can't use an environment variable as it's shared among all thread. Thus always use the api call. Signed-off-by: Olga Kornievskaia <kolga@netapp.com> Reviewed-by: Steve Dickson <steved@redhat.com> --- utils/gssd/gssd_proc.c | 12 +++++++++-- utils/gssd/krb5_util.c | 56 +++++++++----------------------------------------- utils/gssd/krb5_util.h | 1 - 3 files changed, 20 insertions(+), 49 deletions(-) diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c index de8dc07..14298d0 100644 --- a/utils/gssd/gssd_proc.c +++ b/utils/gssd/gssd_proc.c @@ -547,7 +547,15 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname, goto out; } for (ccname = credlist; ccname && *ccname; ccname++) { - gssd_setup_krb5_machine_gss_ccache(*ccname); + u_int min_stat; + + if (gss_krb5_ccache_name(&min_stat, *ccname, NULL) != + GSS_S_COMPLETE) { + printerr(1, "WARNING: gss_krb5_ccache_name " + "with name '%s' failed (%s)\n", + *ccname, error_message(min_stat)); + continue; + } if ((create_auth_rpc_client(clp, tgtname, rpc_clnt, &auth, uid, AUTHTYPE_KRB5, @@ -568,7 +576,7 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname, "recreate cache for server %s\n", clp->servername); } else { - printerr(1, "WARNING: Failed to create machine" + printerr(0, "ERROR: Failed to create machine" "krb5 context with any credentials" "cache for server %s\n", clp->servername); diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c index 3328696..7b74ab3 100644 --- a/utils/gssd/krb5_util.c +++ b/utils/gssd/krb5_util.c @@ -468,37 +468,6 @@ gssd_get_single_krb5_cred(krb5_context context, } /* - * Depending on the version of Kerberos, we either need to use - * a private function, or simply set the environment variable. - */ -static void -gssd_set_krb5_ccache_name(char *ccname) -{ -#ifdef USE_GSS_KRB5_CCACHE_NAME - u_int maj_stat, min_stat; - - printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n", - ccname); - maj_stat = gss_krb5_ccache_name(&min_stat, ccname, NULL); - if (maj_stat != GSS_S_COMPLETE) { - printerr(0, "WARNING: gss_krb5_ccache_name with " - "name '%s' failed (%s)\n", - ccname, error_message(min_stat)); - } -#else - /* - * Set the KRB5CCNAME environment variable to tell the krb5 code - * which credentials cache to use. (Instead of using the private - * function above for which there is no generic gssapi - * equivalent.) - */ - printerr(3, "using environment variable to select krb5 ccache %s\n", - ccname); - setenv("KRB5CCNAME", ccname, 1); -#endif -} - -/* * Given a principal, find a matching ple structure */ static struct gssd_k5_kt_princ * @@ -1094,6 +1063,7 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern) const char *cctype; struct dirent *d; int err, i, j; + u_int maj_stat, min_stat; printerr(3, "looking for client creds with uid %u for " "server %s in %s\n", uid, servername, dirpattern); @@ -1129,22 +1099,16 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern) printerr(2, "using %s as credentials cache for client with " "uid %u for server %s\n", buf, uid, servername); - gssd_set_krb5_ccache_name(buf); - return 0; -} -/* - * Let the gss code know where to find the machine credentials ccache. - * - * Returns: - * void - */ -void -gssd_setup_krb5_machine_gss_ccache(char *ccname) -{ - printerr(2, "using %s as credentials cache for machine creds\n", - ccname); - gssd_set_krb5_ccache_name(ccname); + printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n", + buf); + maj_stat = gss_krb5_ccache_name(&min_stat, buf, NULL); + if (maj_stat != GSS_S_COMPLETE) { + printerr(0, "ERROR: unable to get user cred cache '%s' " + "failed (%s)\n", buf, error_message(min_stat)); + return maj_stat; + } + return 0; } /* diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h index a319588..d3b0777 100644 --- a/utils/gssd/krb5_util.h +++ b/utils/gssd/krb5_util.h @@ -27,7 +27,6 @@ int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirname); int gssd_get_krb5_machine_cred_list(char ***list); void gssd_free_krb5_machine_cred_list(char **list); -void gssd_setup_krb5_machine_gss_ccache(char *servername); void gssd_destroy_krb5_machine_creds(void); int gssd_refresh_krb5_machine_credential(char *hostname, struct gssd_k5_kt_princ *ple, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name 2016-04-27 16:58 ` [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name Olga Kornievskaia @ 2016-04-28 13:19 ` Jeff Layton 2016-04-28 14:11 ` Kornievskaia, Olga 0 siblings, 1 reply; 16+ messages in thread From: Jeff Layton @ 2016-04-28 13:19 UTC (permalink / raw) To: Olga Kornievskaia, steved; +Cc: linux-nfs On Wed, 2016-04-27 at 12:58 -0400, Olga Kornievskaia wrote: > Previously the location of the credential cache was passed in either > using environment variable KRB5CCNAME or gss_krb5_ccache_name() if > supported. For threaded-gssd, we can't use an environment variable > as it's shared among all thread. Thus always use the api call. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > Reviewed-by: Steve Dickson <steved@redhat.com> > --- > utils/gssd/gssd_proc.c | 12 +++++++++-- > utils/gssd/krb5_util.c | 56 +++++++++----------------------------------------- > utils/gssd/krb5_util.h | 1 - > 3 files changed, 20 insertions(+), 49 deletions(-) > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > index de8dc07..14298d0 100644 > --- a/utils/gssd/gssd_proc.c > +++ b/utils/gssd/gssd_proc.c > @@ -547,7 +547,15 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname, > goto out; > } > for (ccname = credlist; ccname && *ccname; ccname++) { > - gssd_setup_krb5_machine_gss_ccache(*ccname); > + u_int min_stat; > + > + if (gss_krb5_ccache_name(&min_stat, *ccname, NULL) != > + GSS_S_COMPLETE) { > + printerr(1, "WARNING: gss_krb5_ccache_name " > + "with name '%s' failed (%s)\n", > + *ccname, error_message(min_stat)); > + continue; > + } > if ((create_auth_rpc_client(clp, tgtname, rpc_clnt, > &auth, uid, > AUTHTYPE_KRB5, > @@ -568,7 +576,7 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname, > "recreate cache for server %s\n", > clp->servername); > } else { > - printerr(1, "WARNING: Failed to create machine" > + printerr(0, "ERROR: Failed to create machine" > "krb5 context with any credentials" > "cache for server %s\n", > clp->servername); > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > index 3328696..7b74ab3 100644 > --- a/utils/gssd/krb5_util.c > +++ b/utils/gssd/krb5_util.c > @@ -468,37 +468,6 @@ gssd_get_single_krb5_cred(krb5_context context, > } > > /* > - * Depending on the version of Kerberos, we either need to use > - * a private function, or simply set the environment variable. > - */ > -static void > -gssd_set_krb5_ccache_name(char *ccname) > -{ > -#ifdef USE_GSS_KRB5_CCACHE_NAME > - u_int maj_stat, min_stat; > - > - printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n", > - ccname); > - maj_stat = gss_krb5_ccache_name(&min_stat, ccname, NULL); > - if (maj_stat != GSS_S_COMPLETE) { > - printerr(0, "WARNING: gss_krb5_ccache_name with " > - "name '%s' failed (%s)\n", > - ccname, error_message(min_stat)); > - } > -#else > - /* > - * Set the KRB5CCNAME environment variable to tell the krb5 code > - * which credentials cache to use. (Instead of using the private > - * function above for which there is no generic gssapi > - * equivalent.) > - */ > - printerr(3, "using environment variable to select krb5 ccache %s\n", > - ccname); > - setenv("KRB5CCNAME", ccname, 1); > -#endif > -} > - > -/* > * Given a principal, find a matching ple structure > */ > static struct gssd_k5_kt_princ * > @@ -1094,6 +1063,7 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern) > const char *cctype; > struct dirent *d; > int err, i, j; > + u_int maj_stat, min_stat; > > printerr(3, "looking for client creds with uid %u for " > "server %s in %s\n", uid, servername, dirpattern); > @@ -1129,22 +1099,16 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern) > > printerr(2, "using %s as credentials cache for client with " > "uid %u for server %s\n", buf, uid, servername); > - gssd_set_krb5_ccache_name(buf); > - return 0; > -} > > -/* > - * Let the gss code know where to find the machine credentials ccache. > - * > - * Returns: > - * void > - */ > -void > -gssd_setup_krb5_machine_gss_ccache(char *ccname) > -{ > - printerr(2, "using %s as credentials cache for machine creds\n", > - ccname); > - gssd_set_krb5_ccache_name(ccname); > + printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n", > + buf); > + maj_stat = gss_krb5_ccache_name(&min_stat, buf, NULL); > + if (maj_stat != GSS_S_COMPLETE) { > + printerr(0, "ERROR: unable to get user cred cache '%s' " > + "failed (%s)\n", buf, error_message(min_stat)); > + return maj_stat; > + } > + return 0; > } > > /* > diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h > index a319588..d3b0777 100644 > --- a/utils/gssd/krb5_util.h > +++ b/utils/gssd/krb5_util.h > @@ -27,7 +27,6 @@ int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, > char *dirname); > int gssd_get_krb5_machine_cred_list(char ***list); > void gssd_free_krb5_machine_cred_list(char **list); > -void gssd_setup_krb5_machine_gss_ccache(char *servername); > void gssd_destroy_krb5_machine_creds(void); > int gssd_refresh_krb5_machine_credential(char *hostname, > struct gssd_k5_kt_princ *ple, Looks fine to me, but I wonder why this env var frobbing is there. Did old MIT krb5 libs lack that function? In any case, they seem to have it now, so I'm fine with this. Also, I think you can probably remove USE_GSS_KRB5_CCACHE_NAME altogether as I don't think it'll be used after this patch. Reviewed-by: Jeff Layton <jlayton@poochiereds.net> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name 2016-04-28 13:19 ` Jeff Layton @ 2016-04-28 14:11 ` Kornievskaia, Olga 2016-04-28 20:43 ` Steve Dickson 0 siblings, 1 reply; 16+ messages in thread From: Kornievskaia, Olga @ 2016-04-28 14:11 UTC (permalink / raw) To: Jeff Layton; +Cc: steved, linux-nfs DQo+IE9uIEFwciAyOCwgMjAxNiwgYXQgOToxOSBBTSwgSmVmZiBMYXl0b24gPGpsYXl0b25AcG9v Y2hpZXJlZHMubmV0PiB3cm90ZToNCj4gDQo+IE9uIFdlZCwgMjAxNi0wNC0yNyBhdCAxMjo1OCAt MDQwMCwgT2xnYSBLb3JuaWV2c2thaWEgd3JvdGU6DQo+PiBQcmV2aW91c2x5IHRoZSBsb2NhdGlv biBvZiB0aGUgY3JlZGVudGlhbCBjYWNoZSB3YXMgcGFzc2VkIGluIGVpdGhlcg0KPj4gdXNpbmcg ZW52aXJvbm1lbnQgdmFyaWFibGUgS1JCNUNDTkFNRSBvciBnc3Nfa3JiNV9jY2FjaGVfbmFtZSgp IGlmDQo+PiBzdXBwb3J0ZWQuIEZvciB0aHJlYWRlZC1nc3NkLCB3ZSBjYW4ndCB1c2UgYW4gZW52 aXJvbm1lbnQgdmFyaWFibGUNCj4+IGFzIGl0J3Mgc2hhcmVkIGFtb25nIGFsbCB0aHJlYWQuIFRo dXMgYWx3YXlzIHVzZSB0aGUgYXBpIGNhbGwuDQo+PiANCj4+IFNpZ25lZC1vZmYtYnk6IE9sZ2Eg S29ybmlldnNrYWlhIDxrb2xnYUBuZXRhcHAuY29tPg0KPj4gUmV2aWV3ZWQtYnk6IFN0ZXZlIERp Y2tzb24gPHN0ZXZlZEByZWRoYXQuY29tPg0KPj4gLS0tDQo+PiAgdXRpbHMvZ3NzZC9nc3NkX3By b2MuYyB8IDEyICsrKysrKysrKy0tDQo+PiAgdXRpbHMvZ3NzZC9rcmI1X3V0aWwuYyB8IDU2ICsr KysrKysrKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+PiAgdXRp bHMvZ3NzZC9rcmI1X3V0aWwuaCB8ICAxIC0NCj4+ICAzIGZpbGVzIGNoYW5nZWQsIDIwIGluc2Vy dGlvbnMoKyksIDQ5IGRlbGV0aW9ucygtKQ0KPj4gDQo+PiBkaWZmIC0tZ2l0IGEvdXRpbHMvZ3Nz ZC9nc3NkX3Byb2MuYyBiL3V0aWxzL2dzc2QvZ3NzZF9wcm9jLmMNCj4+IGluZGV4IGRlOGRjMDcu LjE0Mjk4ZDAgMTAwNjQ0DQo+PiAtLS0gYS91dGlscy9nc3NkL2dzc2RfcHJvYy5jDQo+PiArKysg Yi91dGlscy9nc3NkL2dzc2RfcHJvYy5jDQo+PiBAQCAtNTQ3LDcgKzU0NywxNSBAQCBrcmI1X3Vz ZV9tYWNoaW5lX2NyZWRzKHN0cnVjdCBjbG50X2luZm8gKmNscCwgdWlkX3QgdWlkLCBjaGFyICp0 Z3RuYW1lLA0KPj4gIAkJCWdvdG8gb3V0Ow0KPj4gIAkJfQ0KPj4gIAkJZm9yIChjY25hbWUgPSBj cmVkbGlzdDsgY2NuYW1lICYmICpjY25hbWU7IGNjbmFtZSsrKSB7DQo+PiAtCQkJZ3NzZF9zZXR1 cF9rcmI1X21hY2hpbmVfZ3NzX2NjYWNoZSgqY2NuYW1lKTsNCj4+ICsJCQl1X2ludCBtaW5fc3Rh dDsNCj4+ICsNCj4+ICsJCQlpZiAoZ3NzX2tyYjVfY2NhY2hlX25hbWUoJm1pbl9zdGF0LCAqY2Nu YW1lLCBOVUxMKSAhPQ0KPj4gKwkJCQkJR1NTX1NfQ09NUExFVEUpIHsNCj4+ICsJCQkJcHJpbnRl cnIoMSwgIldBUk5JTkc6IGdzc19rcmI1X2NjYWNoZV9uYW1lICINCj4+ICsJCQkJCSAid2l0aCBu YW1lICclcycgZmFpbGVkICglcylcbiIsDQo+PiArCQkJCQkgKmNjbmFtZSwgZXJyb3JfbWVzc2Fn ZShtaW5fc3RhdCkpOw0KPj4gKwkJCQljb250aW51ZTsNCj4+ICsJCQl9DQo+PiAgCQkJaWYgKChj cmVhdGVfYXV0aF9ycGNfY2xpZW50KGNscCwgdGd0bmFtZSwgcnBjX2NsbnQsDQo+PiAgCQkJCQkJ JmF1dGgsIHVpZCwNCj4+ICAJCQkJCQlBVVRIVFlQRV9LUkI1LA0KPj4gQEAgLTU2OCw3ICs1NzYs NyBAQCBrcmI1X3VzZV9tYWNoaW5lX2NyZWRzKHN0cnVjdCBjbG50X2luZm8gKmNscCwgdWlkX3Qg dWlkLCBjaGFyICp0Z3RuYW1lLA0KPj4gIAkJCQkJICJyZWNyZWF0ZSBjYWNoZSBmb3Igc2VydmVy ICVzXG4iLA0KPj4gIAkJCQkJY2xwLT5zZXJ2ZXJuYW1lKTsNCj4+ICAJCQl9IGVsc2Ugew0KPj4g LQkJCQlwcmludGVycigxLCAiV0FSTklORzogRmFpbGVkIHRvIGNyZWF0ZSBtYWNoaW5lIg0KPj4g KwkJCQlwcmludGVycigwLCAiRVJST1I6IEZhaWxlZCB0byBjcmVhdGUgbWFjaGluZSINCj4+ICAJ CQkJCSAia3JiNSBjb250ZXh0IHdpdGggYW55IGNyZWRlbnRpYWxzIg0KPj4gIAkJCQkJICJjYWNo ZSBmb3Igc2VydmVyICVzXG4iLA0KPj4gIAkJCQkJY2xwLT5zZXJ2ZXJuYW1lKTsNCj4+IGRpZmYg LS1naXQgYS91dGlscy9nc3NkL2tyYjVfdXRpbC5jIGIvdXRpbHMvZ3NzZC9rcmI1X3V0aWwuYw0K Pj4gaW5kZXggMzMyODY5Ni4uN2I3NGFiMyAxMDA2NDQNCj4+IC0tLSBhL3V0aWxzL2dzc2Qva3Ji NV91dGlsLmMNCj4+ICsrKyBiL3V0aWxzL2dzc2Qva3JiNV91dGlsLmMNCj4+IEBAIC00NjgsMzcg KzQ2OCw2IEBAIGdzc2RfZ2V0X3NpbmdsZV9rcmI1X2NyZWQoa3JiNV9jb250ZXh0IGNvbnRleHQs DQo+PiAgfQ0KPj4gIA0KPj4gIC8qDQo+PiAtICogRGVwZW5kaW5nIG9uIHRoZSB2ZXJzaW9uIG9m IEtlcmJlcm9zLCB3ZSBlaXRoZXIgbmVlZCB0byB1c2UNCj4+IC0gKiBhIHByaXZhdGUgZnVuY3Rp b24sIG9yIHNpbXBseSBzZXQgdGhlIGVudmlyb25tZW50IHZhcmlhYmxlLg0KPj4gLSAqLw0KPj4g LXN0YXRpYyB2b2lkDQo+PiAtZ3NzZF9zZXRfa3JiNV9jY2FjaGVfbmFtZShjaGFyICpjY25hbWUp DQo+PiAtew0KPj4gLSNpZmRlZiBVU0VfR1NTX0tSQjVfQ0NBQ0hFX05BTUUNCj4+IC0JdV9pbnQJ bWFqX3N0YXQsIG1pbl9zdGF0Ow0KPj4gLQ0KPj4gLQlwcmludGVycigzLCAidXNpbmcgZ3NzX2ty YjVfY2NhY2hlX25hbWUgdG8gc2VsZWN0IGtyYjUgY2NhY2hlICVzXG4iLA0KPj4gLQkJIGNjbmFt ZSk7DQo+PiAtCW1hal9zdGF0ID0gZ3NzX2tyYjVfY2NhY2hlX25hbWUoJm1pbl9zdGF0LCBjY25h bWUsIE5VTEwpOw0KPj4gLQlpZiAobWFqX3N0YXQgIT0gR1NTX1NfQ09NUExFVEUpIHsNCj4+IC0J CXByaW50ZXJyKDAsICJXQVJOSU5HOiBnc3Nfa3JiNV9jY2FjaGVfbmFtZSB3aXRoICINCj4+IC0J CQkibmFtZSAnJXMnIGZhaWxlZCAoJXMpXG4iLA0KPj4gLQkJCWNjbmFtZSwgZXJyb3JfbWVzc2Fn ZShtaW5fc3RhdCkpOw0KPj4gLQl9DQo+PiAtI2Vsc2UNCj4+IC0JLyoNCj4+IC0JICogU2V0IHRo ZSBLUkI1Q0NOQU1FIGVudmlyb25tZW50IHZhcmlhYmxlIHRvIHRlbGwgdGhlIGtyYjUgY29kZQ0K Pj4gLQkgKiB3aGljaCBjcmVkZW50aWFscyBjYWNoZSB0byB1c2UuICAoSW5zdGVhZCBvZiB1c2lu ZyB0aGUgcHJpdmF0ZQ0KPj4gLQkgKiBmdW5jdGlvbiBhYm92ZSBmb3Igd2hpY2ggdGhlcmUgaXMg bm8gZ2VuZXJpYyBnc3NhcGkNCj4+IC0JICogZXF1aXZhbGVudC4pDQo+PiAtCSAqLw0KPj4gLQlw cmludGVycigzLCAidXNpbmcgZW52aXJvbm1lbnQgdmFyaWFibGUgdG8gc2VsZWN0IGtyYjUgY2Nh Y2hlICVzXG4iLA0KPj4gLQkJIGNjbmFtZSk7DQo+PiAtCXNldGVudigiS1JCNUNDTkFNRSIsIGNj bmFtZSwgMSk7DQo+PiAtI2VuZGlmDQo+PiAtfQ0KPj4gLQ0KPj4gLS8qDQo+PiAgICogR2l2ZW4g YSBwcmluY2lwYWwsIGZpbmQgYSBtYXRjaGluZyBwbGUgc3RydWN0dXJlDQo+PiAgICovDQo+PiAg c3RhdGljIHN0cnVjdCBnc3NkX2s1X2t0X3ByaW5jICoNCj4+IEBAIC0xMDk0LDYgKzEwNjMsNyBA QCBnc3NkX3NldHVwX2tyYjVfdXNlcl9nc3NfY2NhY2hlKHVpZF90IHVpZCwgY2hhciAqc2VydmVy bmFtZSwgY2hhciAqZGlycGF0dGVybikNCj4+ICAJY29uc3QgY2hhcgkJKmNjdHlwZTsNCj4+ICAJ c3RydWN0IGRpcmVudAkJKmQ7DQo+PiAgCWludAkJCWVyciwgaSwgajsNCj4+ICsJdV9pbnQJCQlt YWpfc3RhdCwgbWluX3N0YXQ7DQo+PiAgDQo+PiAgCXByaW50ZXJyKDMsICJsb29raW5nIGZvciBj bGllbnQgY3JlZHMgd2l0aCB1aWQgJXUgZm9yICINCj4+ICAJCSAgICAic2VydmVyICVzIGluICVz XG4iLCB1aWQsIHNlcnZlcm5hbWUsIGRpcnBhdHRlcm4pOw0KPj4gQEAgLTExMjksMjIgKzEwOTks MTYgQEAgZ3NzZF9zZXR1cF9rcmI1X3VzZXJfZ3NzX2NjYWNoZSh1aWRfdCB1aWQsIGNoYXIgKnNl cnZlcm5hbWUsIGNoYXIgKmRpcnBhdHRlcm4pDQo+PiAgDQo+PiAgCXByaW50ZXJyKDIsICJ1c2lu ZyAlcyBhcyBjcmVkZW50aWFscyBjYWNoZSBmb3IgY2xpZW50IHdpdGggIg0KPj4gIAkJICAgICJ1 aWQgJXUgZm9yIHNlcnZlciAlc1xuIiwgYnVmLCB1aWQsIHNlcnZlcm5hbWUpOw0KPj4gLQlnc3Nk X3NldF9rcmI1X2NjYWNoZV9uYW1lKGJ1Zik7DQo+PiAtCXJldHVybiAwOw0KPj4gLX0NCj4+ICAN Cj4+IC0vKg0KPj4gLSAqIExldCB0aGUgZ3NzIGNvZGUga25vdyB3aGVyZSB0byBmaW5kIHRoZSBt YWNoaW5lIGNyZWRlbnRpYWxzIGNjYWNoZS4NCj4+IC0gKg0KPj4gLSAqIFJldHVybnM6DQo+PiAt ICoJdm9pZA0KPj4gLSAqLw0KPj4gLXZvaWQNCj4+IC1nc3NkX3NldHVwX2tyYjVfbWFjaGluZV9n c3NfY2NhY2hlKGNoYXIgKmNjbmFtZSkNCj4+IC17DQo+PiAtCXByaW50ZXJyKDIsICJ1c2luZyAl cyBhcyBjcmVkZW50aWFscyBjYWNoZSBmb3IgbWFjaGluZSBjcmVkc1xuIiwNCj4+IC0JCSBjY25h bWUpOw0KPj4gLQlnc3NkX3NldF9rcmI1X2NjYWNoZV9uYW1lKGNjbmFtZSk7DQo+PiArCXByaW50 ZXJyKDMsICJ1c2luZyBnc3Nfa3JiNV9jY2FjaGVfbmFtZSB0byBzZWxlY3Qga3JiNSBjY2FjaGUg JXNcbiIsDQo+PiArCQkgYnVmKTsNCj4+ICsJbWFqX3N0YXQgPSBnc3Nfa3JiNV9jY2FjaGVfbmFt ZSgmbWluX3N0YXQsIGJ1ZiwgTlVMTCk7DQo+PiArCWlmIChtYWpfc3RhdCAhPSBHU1NfU19DT01Q TEVURSkgew0KPj4gKwkJcHJpbnRlcnIoMCwgIkVSUk9SOiB1bmFibGUgdG8gZ2V0IHVzZXIgY3Jl ZCBjYWNoZSAnJXMnICINCj4+ICsJCQkgImZhaWxlZCAoJXMpXG4iLCBidWYsIGVycm9yX21lc3Nh Z2UobWluX3N0YXQpKTsNCj4+ICsJCXJldHVybiBtYWpfc3RhdDsNCj4+ICsJfQ0KPj4gKwlyZXR1 cm4gMDsNCj4+ICB9DQo+PiAgDQo+PiAgLyoNCj4+IGRpZmYgLS1naXQgYS91dGlscy9nc3NkL2ty YjVfdXRpbC5oIGIvdXRpbHMvZ3NzZC9rcmI1X3V0aWwuaA0KPj4gaW5kZXggYTMxOTU4OC4uZDNi MDc3NyAxMDA2NDQNCj4+IC0tLSBhL3V0aWxzL2dzc2Qva3JiNV91dGlsLmgNCj4+ICsrKyBiL3V0 aWxzL2dzc2Qva3JiNV91dGlsLmgNCj4+IEBAIC0yNyw3ICsyNyw2IEBAIGludCBnc3NkX3NldHVw X2tyYjVfdXNlcl9nc3NfY2NhY2hlKHVpZF90IHVpZCwgY2hhciAqc2VydmVybmFtZSwNCj4+ICAJ CQkJICAgICBjaGFyICpkaXJuYW1lKTsNCj4+ICBpbnQgIGdzc2RfZ2V0X2tyYjVfbWFjaGluZV9j cmVkX2xpc3QoY2hhciAqKipsaXN0KTsNCj4+ICB2b2lkIGdzc2RfZnJlZV9rcmI1X21hY2hpbmVf Y3JlZF9saXN0KGNoYXIgKipsaXN0KTsNCj4+IC12b2lkIGdzc2Rfc2V0dXBfa3JiNV9tYWNoaW5l X2dzc19jY2FjaGUoY2hhciAqc2VydmVybmFtZSk7DQo+PiAgdm9pZCBnc3NkX2Rlc3Ryb3lfa3Ji NV9tYWNoaW5lX2NyZWRzKHZvaWQpOw0KPj4gIGludCAgZ3NzZF9yZWZyZXNoX2tyYjVfbWFjaGlu ZV9jcmVkZW50aWFsKGNoYXIgKmhvc3RuYW1lLA0KPj4gIAkJCQkJICBzdHJ1Y3QgZ3NzZF9rNV9r dF9wcmluYyAqcGxlLCANCj4gDQo+IExvb2tzIGZpbmUgdG8gbWUsIGJ1dCBJIHdvbmRlciB3aHkg dGhpcyBlbnYgdmFyIGZyb2JiaW5nIGlzIHRoZXJlLiBEaWQNCj4gb2xkIE1JVCBrcmI1IGxpYnMg bGFjayB0aGF0IGZ1bmN0aW9uPyBJbiBhbnkgY2FzZSwgdGhleSBzZWVtIHRvIGhhdmUgaXQNCj4g bm93LCBzbyBJJ20gZmluZSB3aXRoIHRoaXMuDQoNCkNvcnJlY3QgcHJpb3IgdG8gc29tZSB2ZXJz aW9uIHRoZXJlIHdhcyBubyBzdWNoIGZ1bmN0aW9uIHRoZXJlLiANCg0KPiBBbHNvLCBJIHRoaW5r IHlvdSBjYW4gcHJvYmFibHkgcmVtb3ZlIFVTRV9HU1NfS1JCNV9DQ0FDSEVfTkFNRQ0KPiBhbHRv Z2V0aGVyIGFzIEkgZG9uJ3QgdGhpbmsgaXQnbGwgYmUgdXNlZCBhZnRlciB0aGlzIHBhdGNoLg0K DQpJIHRoaW5rIGl0IGlzIHJlbW92ZWTigKYgOi0vIGdyZXAgZm9yIENDTkFNRSBjb21lcyB1cCBl bXB0eS4NCg0KPiANCj4gUmV2aWV3ZWQtYnk6IEplZmYgTGF5dG9uIDxqbGF5dG9uQHBvb2NoaWVy ZWRzLm5ldD4NCg0K ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name 2016-04-28 14:11 ` Kornievskaia, Olga @ 2016-04-28 20:43 ` Steve Dickson 0 siblings, 0 replies; 16+ messages in thread From: Steve Dickson @ 2016-04-28 20:43 UTC (permalink / raw) To: Kornievskaia, Olga, Jeff Layton; +Cc: linux-nfs On 04/28/2016 10:11 AM, Kornievskaia, Olga wrote: >> Also, I think you can probably remove USE_GSS_KRB5_CCACHE_NAME Good call... Its gone! >> > altogether as I don't think it'll be used after this patch. > I think it is removed… :-/ grep for CCNAME comes up empty. > Its in the aclocal/kerberos5.m4 file... your favorite files! 8-) steved. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v5 0/3] adding pthread support to gssd 2016-04-27 16:58 [RFC PATCH v5 0/3] adding pthread support to gssd Olga Kornievskaia ` (2 preceding siblings ...) 2016-04-27 16:58 ` [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name Olga Kornievskaia @ 2016-04-29 14:49 ` Steve Dickson 2016-04-29 14:57 ` Kornievskaia, Olga 3 siblings, 1 reply; 16+ messages in thread From: Steve Dickson @ 2016-04-29 14:49 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs On 04/27/2016 12:58 PM, Olga Kornievskaia wrote: > Adding support for multi-threaded gssd and allow for parallel upcalls. > For each upcall create a thread, detach to allow for automatic cleanup. > Communicate location of credential cache with gss api function instead > of an environmental variable. Use syscall api instead of libc setuid() > to change user identity. > > Previously parent thread after detaching from child thread would call > yield to let the child thread run and consume the event from the fd. > Instead, use synchronization variables to signal parent after reading > from fd and have parent thread wait for the event. > > Olga Kornievskaia (3): > gssd: use pthreads to handle upcalls > gssd: using syscalls directly to change thread's identity > gssd: always call gss_krb5_ccache_name > > aclocal/libpthread.m4 | 13 ++++++++++ > configure.ac | 3 +++ > utils/gssd/Makefile.am | 3 ++- > utils/gssd/gssd.c | 44 ++++++++++++++++++++++++++++++--- > utils/gssd/gssd.h | 5 ++++ > utils/gssd/gssd_proc.c | 66 +++++++++++++++++++++++--------------------------- > utils/gssd/krb5_util.c | 59 ++++++++++---------------------------------- > utils/gssd/krb5_util.h | 1 - > 8 files changed, 106 insertions(+), 88 deletions(-) > create mode 100644 aclocal/libpthread.m4 > All three patches have been committed... thanks for doing this work!! steved. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v5 0/3] adding pthread support to gssd 2016-04-29 14:49 ` [RFC PATCH v5 0/3] adding pthread support to gssd Steve Dickson @ 2016-04-29 14:57 ` Kornievskaia, Olga 0 siblings, 0 replies; 16+ messages in thread From: Kornievskaia, Olga @ 2016-04-29 14:57 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs > On Apr 29, 2016, at 10:49 AM, Steve Dickson <SteveD@redhat.com> wrote: > > > > On 04/27/2016 12:58 PM, Olga Kornievskaia wrote: >> Adding support for multi-threaded gssd and allow for parallel upcalls. >> For each upcall create a thread, detach to allow for automatic cleanup. >> Communicate location of credential cache with gss api function instead >> of an environmental variable. Use syscall api instead of libc setuid() >> to change user identity. >> >> Previously parent thread after detaching from child thread would call >> yield to let the child thread run and consume the event from the fd. >> Instead, use synchronization variables to signal parent after reading >> from fd and have parent thread wait for the event. >> >> Olga Kornievskaia (3): >> gssd: use pthreads to handle upcalls >> gssd: using syscalls directly to change thread's identity >> gssd: always call gss_krb5_ccache_name >> >> aclocal/libpthread.m4 | 13 ++++++++++ >> configure.ac | 3 +++ >> utils/gssd/Makefile.am | 3 ++- >> utils/gssd/gssd.c | 44 ++++++++++++++++++++++++++++++--- >> utils/gssd/gssd.h | 5 ++++ >> utils/gssd/gssd_proc.c | 66 +++++++++++++++++++++++--------------------------- >> utils/gssd/krb5_util.c | 59 ++++++++++---------------------------------- >> utils/gssd/krb5_util.h | 1 - >> 8 files changed, 106 insertions(+), 88 deletions(-) >> create mode 100644 aclocal/libpthread.m4 >> > All three patches have been committed... thanks for doing this work!! Steve, Jeff thanks for the reviews. I’m onto the next step of making a pool of threads. > > steved. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-04-30 17:29 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-27 16:58 [RFC PATCH v5 0/3] adding pthread support to gssd Olga Kornievskaia
2016-04-27 16:58 ` [PATCH v5 1/3] gssd: use pthreads to handle upcalls Olga Kornievskaia
2016-04-27 17:39 ` Steve Dickson
2016-04-28 13:13 ` Jeff Layton
2016-04-28 14:07 ` Kornievskaia, Olga
2016-04-30 12:29 ` Jeff Layton
2016-04-30 16:52 ` Olga Kornievskaia
[not found] ` <BF14CEF3-6B57-43FD-9226-00A25886844F@netapp.com>
2016-04-30 17:29 ` Jeff Layton
2016-04-27 16:58 ` [PATCH v5 2/3] gssd: using syscalls directly to change thread's identity Olga Kornievskaia
2016-04-28 13:14 ` Jeff Layton
2016-04-27 16:58 ` [PATCH v5 3/3] gssd: always call gss_krb5_ccache_name Olga Kornievskaia
2016-04-28 13:19 ` Jeff Layton
2016-04-28 14:11 ` Kornievskaia, Olga
2016-04-28 20:43 ` Steve Dickson
2016-04-29 14:49 ` [RFC PATCH v5 0/3] adding pthread support to gssd Steve Dickson
2016-04-29 14:57 ` Kornievskaia, Olga
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.